intel / ModernFW

Other
107 stars 18 forks source link

Alternate C style #7

Open vincent-j-zimmer opened 5 years ago

vincent-j-zimmer commented 5 years ago

Explore going from Camel Case EDKII style https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C naming to Indian Hill https://www2.cs.arizona.edu/~mccann/cstyle.html in a similar manner to U-boot's UEFI implementation (e.g., efi_handle_t in https://github.com/u-boot/u-boot/blob/master/include/efi.h)

MichaelARothman commented 5 years ago

With regards to the Camel Case vs Indian Hill topic, I have lots of empathy for switching, especially making it easier for newbies to "get" what they're looking at, since Indian Hill is likely better understood/known.

But the concern would be about divorcing ourselves from an EDK2 upstream - UEFI spec also has lots of examples purely from Camel Case perspective. Diff'ing with EDK2 would be a nightmare.

Granted, a tool COULD be created to facilitate swizzling code from one format to the other and I'd posit it might make sense the repo contains Camel Case for diff'ing/maintenance.

We need more folks who would be the participating demographic (OS folks?) to see if this is even a serious concern.

Come to think of it, we probably need to brainstorm the biggest issues and prioritize what they are and construct plans to address each in an order.

ronstory commented 5 years ago

I'm going to paraphrase very smart man and say "Don't be encumbered by a codebase, go do something wonderful". So let not worry about upstreaming the result to EDK2 but take risks and dream of codebase to support future OSes for the next 20 years. Who knows, the results of this modernization may not even be written in C. ;^)

marksdoran commented 5 years ago

So far as I understand it, the reason to raise this issue comes down to the question of how readable and familiar ModernFW could would be to different audiences. In particular it's asserted that there are more people in the world familiar with Indian Hill (for example because of UNIX/Linux work) as compared to those familiar with Camel Case (from venues like Windows development). Given that the intent is to make it easy to bring new people into this project then something familiar is a lower bar.

I submit that merely changing the syntax used for labels of various sorts and changing code layout rules are changes that wouldn't really make TianoCore code more understandable. Fundamentally, what's familiar or not is the idiom of code.

Anyone that's worked on code isn't going to have a hard time working out what instance_count does as compared to InstanceCount; similar equivalence for comment markings and things like block brace placements and so forth.

More important than that it's things like the ideas of protocols and the system table, how the interfaces are exercised, and instance data with the containment record macro (...etc.) found in EDK2 that really make the difference to how hard it is for unfamiliar newcomers to figure out what code is doing when your scope of looking exceeds a function or two.

Indian Hill versus Camel Case alone doesn't do much for you against that kind of consideration.

I agree with Mr. Rothman's observation too -- changing syntactic things would significantly impact ability to upstream any results beck to the EDK2 upstream. Depnding on direction, that's either a "don't care" or a huge issue.

In any case, to really make the code easier for Linux kernel code experts to understand at first encounter I think you need to change coding style and construction idiom.

As a result, I think this issue is only meaningful if we treat it as a referendum on the code base starting point and what if any relationship we believe results should have to the EDK2 project. In other words, to address this issue properly a change would have to be a lot more extensive than just swapping coding style conventions. Which we can do, we just need to understand the implications and avoid doing half the job for no gain in the end.