On Fri, Jul 11, 2014 at 12:42 PM, Bill Meier <[email protected]> wrote:
> 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. > When you try and push a change to gerrit without a Commit-ID, the error message it returns includes the Commit-ID it's expecting, so you can manually git commit --amend and paste that into the footer. > > [1] http://wiki.wireshark.org/Development/SubmittingPatches > ____________________________________________________________ > _______________ > Sent via: Wireshark-dev mailing list <[email protected]> > Archives: http://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev > mailto:[email protected]?subject=unsubscribe >
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
