Re: really stop "really" commits (really!)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Fedora Announce]     [Fedora Kernel]     [Fedora Testing]     [Fedora Formulas]     [Fedora PHP Devel]     [Kernel Development]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [PAM]     [Red Hat Development]     [Gimp]     [Yosemite News]
  Powered by Linux