Hi,

I was reviewing a patch (https://gerrit.fd.io/r/c/vpp/+/45456) and noticed
that there were a lot of lines where no code was changed, but they were
just reformatted to rearrange whitespace. It looked like the maximum width
of a line was increased to 100. I then noticed that the .clang-format file
had 'ColumnLimit: 100' added to it recently.

If you look at https://gerrit.fd.io/r/c/vpp/+/45456 and compare Base to
Patchset 1, the delta of src/vnet/sfdp/sfdp.c is +16 -2 and it's pretty
easy to see what's being changed. Patchset 1 does not contain all of the
formatting changes. If you compare Base to Patchset 5, the delta of
src/vnet/sfdp/sfdp.c is +107 -112 and you have to scroll past a whole lot
of formatting changes before you find the actual code changes.

I presume that src/vnet/sfdp/sfdp.c was added before the change to
.clang-format so it was formatted using a line width of 80 chars
originally. Now that it's being changed, checkstyle is enforcing the new
100 char width on the file. I imagine that minor changes made to other
files will be subject to the same issue - checkstyle will require a bunch
of superfluous reformatting to increase line width to 100. This seems like
it has the potential to make it a little harder to review changes that
people submit since you will have to look very closely at which differences
actually change behavior of the code and try to filter out a lot of noise
resulting from formatting changes.

Aside from the reformatting noise, I also usually find code formatted for
80 chars easier to read since I often am viewing source files in vim on a
terminal window that defaults to 80 chars wide. Though I acknowledge that I
might be in the minority in that regard nowadays.

Does anyone else think this might be a problem?

Thanks,
-Matt
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#26939): https://lists.fd.io/g/vpp-dev/message/26939
Mute This Topic: https://lists.fd.io/mt/118744922/21656
Group Owner: [email protected]
Unsubscribe: https://lists.fd.io/g/vpp-dev/leave/14379924/21656/631435203/xyzzy 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to