Lint the C and H files

[hecko]

I suggest to lint the C and H files using clang-format to have a code consistency. Discussion / feedback appreciated.

Suggested clang-format style to start with:

BasedOnStyle: LLVM
AlignConsecutiveMacros: true
SpacesBeforeTrailingComments: 2
IndentPPDirectives: AfterHash

[WestfW]

I am significantly opposed to gratuitous formatting changes, even for the sake of gaining consistency with some supposed "standard." (when did formatting become a lint thing?)

[hecko]

With Python, but mostly with Golang I believe.

My initial intention was rather to be able to actually run some kind of style formatter to check for code style inconsistencies than to align with some de-facto standard (line LLVM or Google).

Subjectively speaking the code does not look very consistent anyways - I can see several lines which are >100 chars long, but also a documentation text which is aligned to 60 chars. There are also a few lines which look odd (again, subjectively), like the following one:

unsigned const int __attribute__((section(".version")))
optiboot_version = 256*(OPTIBOOT_MAJVER + OPTIBOOT_CUSTOMVER) + OPTIBOOT_MINVER;

Also some preprocessor directives (which are used a lot) are indented and some are not, which is very confusing.

I understand, that coding style is a very personal issue, but having an option to either use tools to check style consistency and lose a bit of freedom or having complete freedom to set the style as required, but have no way to check the consistency of my changes I would personally choose the option to be able to use tools to check the style. Again, IMHO.

[WestfW]

MCUDude did code reformatting, and the results are that it's very difficult to tell what other differences there are between his optiboot_flash and the current optiboot parent (https://github.com/Optiboot/optiboot/commit/564372673a5167b3e30da1f3e4e29916321a9b20 was supposed to reset some of those diffs.) Likewise, the LGT source seems to have been forked off at around version 5 (before the patch above), so IT'S hard to compare.

The inconsistencies are mostly a matter of timing, while I'd normally try to preserve the original style of code, some things (like the indented pre-processor directives) are useful enough to use in "new code" even if they weren't done originally (but not worth re-formatting the old code.)

Which lint tool are you using?

[hecko]

I was planning to use clang-format with the following baseline .clang-format file content:

BasedOnStyle: LLVM
AlignConsecutiveMacros: true
SpacesBeforeTrailingComments: 2
IndentPPDirectives: AfterHash
ColumnLimit: 140

And run it as:

clang-format -style=file -i *.h *.c

I can totally see the issue with keeping-up with the changes, but the current state of the code (running the clang-format) looks like it has so many different ways to express the same thing....I don't even know what the baseline for "this style" is. Therefore designing a proper style definition is not possible for me; hence I have proposed a de-facto standard from LLVM project with minor tweaks.

I personally like the indentation of preprocessor stanzas, but the current optiboot.c uses three different ways to actually indent / not indent / differently_indent these so Im not sure which style was intended to be used, but not pursued because of "timing".

I totally get your position - maybe its a matter of release. Release version 9.0.0 and version 9.1.0 can ONLY have style consistency applied - the style which is the most likely not to change the code too much, but still make the code internally consistent. I dont know, at this stage what style that would be, but I strongly recommend using clang-format with some kind of baseline style (LLVM?) and as many as possible style rule tweaks using the configuration file to match the current code as much as possible. Its a lot of trial and error, but I personally think its worth the effort.

One of the great benefits of this is, that other forked repos can easily apply the same style using clang-format to their codebases and can see the diff agains optiboot in a much more consistent manner. Not to mention easy to backport changes from forks back to optiboot.

[hecko]

@WestfW any more thoughts on this?

[hecko]

@WestfW any thoughts on this or shall I close this as N/A for optiboot?

[hecko]

Unfortunately no response from devs. Closing issue.