Hi Bill,
On Fri, Jul 11, 2014 at 6:42 PM, Bill Meier <wmeier@xxxxxxxxxxx> wrote:
> I've been working with the current 'pre-commit' and have noticed the
> following issues:
at origin, pre-commit script is personnal script to easy launch some
check tools ;-) and there is a lot of issue...
>
> 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 ?
> - ???
it is now fix by Evan and if i remember it is a rand sha1 start with I...
>
> 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').
Yes good idea !
>
> 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.
need to add "exception" for ui/qt about C comment because // is a
valid comment in C++ and checkAPI fail in ui/qt (there is a small hack
in pre-commit check only c and h files but there is some h files in
ui/qt...)
Also there is a other issue when remove file (script try always to
check remove file...)
And i will be nice if it is possible to check the last commit (or a
specificity commit id)
>
> 2. In addition, I propose to add calls to checkhf and fix-encoding-args
> under the make file checkapi targets for dissector files.
Good idea too
>
> ====
>
> 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
> ___________________________________________________________________________
> Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives: http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
