On Fri, 2013-12-13 at 18:42 -0700, T.C. Hollingsworth wrote: > Invariably when adding a patch to a spec, often I forget some detail, > whether it be adding the %patchN macro to %prep or `git add`ing the > patch. It would seem I'm not alone, either. A Google search for e.g. > "site:https://lists.fedoraproject.org/pipermail/scm-commits/ really > apply patch" returns tens of thousands of results! ;-) > > To prevent this from happening in the future, I wrote a little git > pre-commit hook to help out, which I figured I'd share with you all: > http://patches.fedorapeople.org/patchcheck.py > > It verifies that: > - all patches are committed to git > - all patches are applied in %prep > - no unexpanded %patch macros exist in %prep > > If any of the above checks fail, the commit is aborted. Thanks - this is a great idea. I notice that near the top you: # stash any unstaged changes and later clean it up with: # restore unstaged changes There are ways of exiting the script that lead to this cleanup not occurring - either on a specfile parse error, or if an unexpected exception occurs - so perhaps the stash/unstash should be done using a context manager? (or just wrap it all in a big try/finally clause?) Also, perhaps replace the "abort" bool with an error count, and make the exit code be that? (and for that matter have a function to do the sys.stderr.write/set-error dance - but now I'm bikeshedding :) ) Hope this is constructive Dave -- devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/devel Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct
