code formatting

[rgetz]

with so many people working on libiio, It's time to put some formatting rules in place...

I was looking at clang-format, since we can include that as a pull request check, similar to:
https://github.com/pierremoreau/SPIRV-LLVM-Translator/blob/master/utils/check_code_format.sh
(which checks only the diff).

I think I tried to minimize things - but it's alot of current inconsistencies...

Things like screen width:

75 columns : 45 files changed, 3822 insertions(+), 3051 deletions(-)
76 columns : 45 files changed, 3720 insertions(+), 3032 deletions(-)
77 columns : 44 files changed, 3599 insertions(+), 2993 deletions(-)
78 columns : 44 files changed, 3535 insertions(+), 2986 deletions(-)
79 columns : 44 files changed, 3476 insertions(+), 3005 deletions(-)
80 columns : 44 files changed, 3365 insertions(+), 2980 deletions(-)
81 columns : 44 files changed, 3346 insertions(+), 3016 deletions(-)
82 columns : 44 files changed, 3309 insertions(+), 3042 deletions(-)
83 columns : 44 files changed, 3288 insertions(+), 3075 deletions(-)
84 columns : 44 files changed, 3255 insertions(+), 3103 deletions(-)
85 columns : 44 files changed, 3220 insertions(+), 3128 deletions(-)

some are obvious, just based on the number of changes:
AlignAfterOpenBracket: DontAlign 44 files changed, 3365 insertions(+), 2980 deletions(-)
AlignAfterOpenBracket: Align 44 files changed, 3781 insertions(+), 3263 deletions(-)

-ContinuationIndentWidth: 16 44 files changed, 3365 insertions(+), 2980 deletions(-)
+ContinuationIndentWidth: 8 44 files changed, 3672 insertions(+), 3316 deletions(-)

-IndentCaseLabels: true 44 files changed, 3365 insertions(+), 2980 deletions(-)
+IndentCaseLabels: false 44 files changed, 3087 insertions(+), 2755 deletions(-)

-SpaceAfterCStyleCast: true 44 files changed, 3087 insertions(+), 2755 deletions(-)
+SpaceAfterCStyleCast: false 44 files changed, 3289 insertions(+), 2962 deletions(-)

-const char * iio_get_backend(unsigned int index)
+const char *iio_get_backend(unsigned int index)

1. Is this a worthwhile thing?

2. Since there is going to be so much whitespace changes, I'm not sure it really matters which way we pick - Since there are so many kernel developers on the team, this is mostly kernel coding style - we can make it exactly the same if that is what everyone wants...

Thoughts?

[tfcollins]

1. I think this (and static analysis) could make the PR process less work for reviewers
2. It dirties git blame a bit, but I think you can add flags to ignore whitespace changes now. astyle and clang-format are probably the standard now. I think most people already use astyle at the moment.

[rgetz]

White space changes don't remove that much..

git diff | diffstat
43 files changed, 3083 insertions(+), 2750 deletions(-)

% ignore white space chnages
git diff -w | diffstat
40 files changed, 1881 insertions(+), 1548 deletions(-)

The majority come from splitting lines due to length...

-struct iio_buffer * iio_device_create_buffer(const struct iio_device *dev,
-               size_t samples_count, bool cyclic)
+struct iio_buffer *iio_device_create_buffer(
+               const struct iio_device *dev, size_t samples_count, bool cyclic)
 {

is not a white space change...

[rgetz]

but yes - it would make style reviews on PR much easier.

[tfcollins]

Interesting reads:
https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame
https://phabricator.kde.org/T11214

[rgetz]

Very cool.

I agree with the thought , I think it'll make reviews smoother, ease onboarding and free up valuable time for code. Completely agree.

From the first link:

Git 2.23 contains an absolute game changer that is not even mentioned in the release highlights. Fear of polluting the git blame output no longer has to be a blocker for applying style changes in bulk: these commits can now be ignored.

Hoever - Git 2.23 is pretty recent - 16-Aug-2019, most modern distributions would not have that...

debian buster (stable) & Ubuntu Eoan: is 2.20
debian bullseye (testing) & Ubuntu focal (April 23, 2020) is 2.25

so most people won't have those yet, but should soon. Won't help on github, which is Ok in my workflow...

Interested in what others think...

PS. - I tried:
0 columns : 42 files changed, 1890 insertions(+), 1850 deletions
but that does make of mess of some things - combining many lines into a single 130+ char line (from ~100 today)

[dNechita]

Code formatting would be quite useful.
On aditof_sdk we use clang-format. There's a job in CI which checks if new code is properly formatted:
https://github.com/analogdevicesinc/aditof_sdk/blob/4b06e1ef46ff00bab61aec8368f688eac6f949e3/ci/travis/lib.sh#L58

[rgetz]

@dNechita

OK - I will add something - and we can start from there.

Before anything is added - shouldn't we be using the same .clang-format format file for all ADI originated projects? (unless it's a small part of something upstream?)

For developers - it easier if things are at least similar. (tabs/spaces/etc).

How do you see this split across ADI's projects? It can't be each project on it's own.

-Robin

[dNechita]

@dNechita

OK - I will add something - and we can start from there.

Before anything is added - shouldn't we be using the same .clang-format format file for all ADI originated projects? (unless it's a small part of something upstream?)

For developers - it easier if things are at least similar. (tabs/spaces/etc).

How do you see this split across ADI's projects? It can't be each project on it's own.

-Robin

I can see the value in having consistent formatting in our projects but it doesn't make sense to enforce exactly the same formatting options for all projects. A common set of options could be established, like: always use tabs, spaces between binary operators, if statement and '{' on the same line, etc. The examples I've provided are my personal preference but the common set of options should be discussed by the majority.
Each projects should preserve the common options but could add others that don't interfere with the common ones. A comment should be added next to each of the common to make developers aware.
I would keep the number of common options to a minimum thus leaving the projects the flexibility to change their formatting if/when necessary.

[rgetz]

Yeah, I get that is really depends on the language, and project.

I was thinking that we should have:

• a single python one for all our bindings, and python projects.
• a single C one for NO-OS drivers (that should likely match the linux kernel), since many are close, and it will help compare things.
• a Qt project wants to follow the Qt guidelines. https://github.com/qt-creator/qt-creator/tree/master/dist/clangformat which may be different than other C++ projects.
• a project that interfaces with openCL, should be using their file - https://github.com/opencv/opencv_contrib/blob/master/modules/cvv/.clang-format
• but for C projects that ADI/SDG makes that should be mostly the same to identical. I don't seen the point in differences (unless like above, it's fitting into a larger framework).

It shouldn't be developer preference - IMHO - or there is no point to having a "standard".

For projects like the 3DToF, which are multiple things - I think it's OK to have multiple standards in a single project, as long as it's clear why, and in what section. It can't be a mix, of whatever the developer whats to choose. the openCV bindings and examples should use that. The library itself - can use the ADI version.

Open to comments.

[mhennerich]

libiio issue tracker is probably not the proper place to discuss processes for the entire SW team.

In fact we already have coding style standards for the different projects in our process document.

No-OS coding style is similar to Linux style:
https://github.com/analogdevicesinc/no-OS/blob/master/ci/travis/astyle_config

ADI ToF_SDK:
https://github.com/analogdevicesinc/aditof_sdk/blob/master/.clang-format

Scopy has one too.

Libiio - follows Linux coding style so we can probably use:
https://github.com/torvalds/linux/blob/master/.clang-format

[rgetz]

agree - but there isn't really a better (open) place to discuss - is there? :) Plus - this started as a question about what coding standard for libiio we should use,

What you are saying is there is no one standard, and who ever commits to the repo first defines the coding standard for that repo. That doesn't make sense to me.That leaves it to personal preference of the first developer.

[mhennerich]

What you are saying is there is no one standard

Didn't I mention? -

Libiio - follows Linux coding style so we can probably use:
https://github.com/torvalds/linux/blob/master/.clang-format

[rgetz]

What you are saying is there is no one standard

Didn't I mention? -

Is that the desired state? It just makes it harder for everyone... (users and developers).

I agree that this is now a beer discussion, best had when more people are face to face.

For now - the plan is use the linux coding style, I will submit the:

• .clang-format
• a massive pull request which updates all the files.
• checker that works on travis-ci,
• instructions for doing this as part of a pre-commit hook in contribute.md

and close this when that is done.

[mhennerich]

Thanks for taking the lead in establishing a more formal and documented standard.
@adisuciu @dNechita will work together and making sure there is a common standard for C++ projects.

[rgetz]

Nothing like a little work at home (due to world health issues/fears of unknown) to find some time to work on maintenance/infrastructure. :)

[rgetz]

This will be one of the first things to get done for the 0.21 release, before any other code changes...