I've been working with the current 'pre-commit' and have noticed the
following issues:
1. Using the current pre-commit which calls checkAPIs, etc, it
doesn't seem possible to make changes to
certain files (e.g., wsgetopt.c) and submit them to Gerrit.
- The files fail checkAPIs.pl as called from pre-commit
(Error: deprecated/prohibited APIs).
'git commit --no-verify' bypasses pre-commit, but also bypasses
commit-msg and thus the commit message doesn't
have a Change-ID and thus is rejected by Gerrit. [**See below]
Is there a way around this (short of temporarily removing
the pre-commit script) ?
- somehow generate a Gerrit Commit-ID manually ?
- ???
I note that there a a number of (non-dissector) files
which fail the default checkAPIs. Many of the Errors could probably
be fixed, but some look either OK or a bit tricky.
The reason that we don't see these checkAPIs issues with
'make checkapi' is that the checkAPIs for the files which fail
have been commented out (thus sort of saying the Errors are OK).
2. checkhf and fix-encoding-args are being called for all files (not
just dissectors).
====
1. For the above reasons, I propose that pre-commit only do checkAPIs,
checkhf and fix-encoding-args for dissector files (to be determined
in a rather ugly ad-hoc way by seeing if the file is named
"packet-.+\.[hc]" (as is done now with 'checkAPIs -p').
pre-commit would still do the whitespace check for all files.
checkAPIs can be called for all .[hc] files when/if the current
Errors are fixed.
2. In addition, I propose to add calls to checkhf and fix-encoding-args
under the make file checkapi targets for dissector files.
====
Thoughts ?
Bill
[**]
The Wireshark wiki [1] states
"If you must have the whitespace as it is, you can run git commit
--no-verify to avoid the pre-commit and commit-msg hooks. Note: using
--no-verify avoids the commit-msg hook, and thus does not add a
Change-ID automatically to your commit."
Ok: We really don't want to accept commits which don't pass the
whitespace test so this shouldn't be an issue when using
the default pre-commit.
However, the Wiki doesn't say what to do when we really, really
"must have the whitespace" except to say '--no-verify' leaves
us with a commit message with no Commit-ID.
[1] http://wiki.wireshark.org/Development/SubmittingPatches