[LU-2425] wirecheck does not follow new coding style

• Status: Resolved
• Project: Lustre
• Type: Bug
• Priority: Minor
• Reporter: CEA
• Assignee: Bob Glossman (Inactive)
• Resolution: Won't Fix
• Severity: 3

Description

The code generated by wirecheck and wirecheck.c do not follow new coding style (use of space + line too long)

Comments

[Peter Jones]

Bob

Could you please look into this one?

Thanks

Peter

[Bob Glossman (Inactive)]

While it's true that the generated sources like wiretest.c still use spaces instead of tabs as the newest coding conventions recommend, is this really important to fix? It seems to me that the risk of introducing bugs by fiddling with the generator code in wirecheck.c far exceeds the value of exactly matching current conventions. The existing generated code is still perfectly good and valid.

[jacques-charles lafoucriere]

If you use the official and mandatory git/hooks, you cannot commit a patch which does not follow the coding style.
Without the right coding style, a patch does not pass the review.
There are multiple issues that build/checkpatch.pl does not like in the generated code (line too long, space vs tab, missing spaces, ...)

[Andreas Dilger]

Bob, it is an oversight that the wiretest.c and wirecheck.c files do not match the coding style.

I suspect it would be relatively straight forward to fix the wirecheck.c code to match the coding style (use a tab for indentation, add a line break and indent in the middle) and then regenerate the wiretest.c files.

With diff highlighting in Gerrit, it should be relatively straight forward to verify that the commit which updates these files does not introduce any semantic changes.

JC, please note that while the git commit hooks may report warnings and errors with the code style, these do not prevent committing the change. It only prevents committing the change if the commit message itself is incorrect.

[Bob Glossman (Inactive)]

I have revised wirecheck.c and generated new wiretest files that eliminate all the errors and warnings except for the ones for lines over 80 characters. Given the very long casts and offestof() and other complex strings with few spaces, I couldn't figure out good ways to shorten many lines.

Pushing what changes I have for review.

http://review.whamcloud.com/#change,4751

[Andreas Dilger]

It seems this was fixed as part of the HSM patch http://review.whamcloud.com/4736. Are there still bits left in the 4751 patch that are needed, or should this be closed?

[Bob Glossman (Inactive)]

It's been quite a while since I revisited this. It looks to me like all the tabs/spaces issues have been fixed. There are still lines > 80 chars in wiretest.c, but the proposed patch didn't fix those anyway.

I believe this should be closed. I'll go ahead and mark http://review.whamcloud.com/4751 ABANDONED.

[John Fuchs-Chesney (Inactive)]

Partly fixed, but not completely.