Wireshark mailing list archives
Re: Current 'pre-commit' issues
From: Evan Huus <eapache () gmail com>
Date: Fri, 11 Jul 2014 15:59:41 -0400
On Fri, Jul 11, 2014 at 12:42 PM, Bill Meier <wmeier () newsguy com> 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 ?
+1
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 ____________________________________________________________ _______________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Current 'pre-commit' issues Bill Meier (Jul 11)
- Re: Current 'pre-commit' issues Evan Huus (Jul 11)
- Re: Current 'pre-commit' issues Bill Meier (Jul 11)
- Re: Current 'pre-commit' issues Evan Huus (Jul 11)
- Re: Current 'pre-commit' issues Alexis La Goutte (Jul 13)
- Re: Current 'pre-commit' issues Jeff Morriss (Jul 22)
- Re: Current 'pre-commit' issues Evan Huus (Jul 11)