Hi Matt,
The formatting churn in patchset 5 is indeed a side effect of checkstyle 
enforcing the ColumnLimit: 100 introduced in commit 8012ddc35 (build: increase 
column Limit to 100). Since the sfdp files were originally written before that 
change, any touched file gets bulk-reformatted, which drowns out the actual 
functional changes.
The right fix is to split the patch into two separate commits:
1. A pure formatting commit (the 80→100 char reflow, no logic changes)
2. A functional commit with only the real code changes
This way reviewers can skip the formatting commit and focus on what matters.
I'll post an updated patchset with that split shortly.

Jerome

De : [email protected] <[email protected]> de la part de Matthew Smith via 
lists.fd.io <[email protected]>
Date : jeudi, 9 avril 2026 à 17:53
À : vpp-dev <[email protected]>
Objet : [vpp-dev] Side effect of clang-format 'ColumnLimit: 100'

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 (#26940): https://lists.fd.io/g/vpp-dev/message/26940
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