[vtk-developers] New git commit hooks

Dave Partyka dave.partyka at kitware.com
Fri Apr 23 14:24:17 EDT 2010


We definitely are enforcing 78 character or less as I have had to rebase
--amend several commits from others that I cherry-picked onto the release
branch and then tried to push  to origin.

On Fri, Apr 23, 2010 at 2:17 PM, Berk Geveci <berk.geveci at kitware.com>wrote:

> I was just describing the conversation I had with Brad. The idea was
> to have checks only for things that screwed up the history (like
> merging from next to master) or possibly things that could corrupt
> people's repositories (I believe an example for that was screwed up
> line feed). We were not supposed to have commit checks for anything
> you listed. I somehow don't believe that we are enforcing 79 character
> or less. If we are, we should stop. Same with trailing white spaces.
>
> Brad, can you chime in here please.
>
> -berk
>
> On Fri, Apr 23, 2010 at 1:42 PM, Moreland, Kenneth <kmorel at sandia.gov>
> wrote:
> > That doesn’t make any sense Berk.  As it is, there already exists several
> > hooks that restrict the commits that you can place, so the problem you
> > describe can already happen as it is.  This is why the advice is to
> install
> > the hooks in your local repository.
> >
> > None of the current checks I found in these hooks are fatal or close to
> > fatal.  On checks to make sure that there is no whitespace at the end of
> a
> > line.  How is that close to fatal?  Another makes sure that the first
> line
> > of your commit message is 79 characters or less.  Will my web browser die
> if
> > it encounters an 80 character commit message?
> >
> > I consider tab removal more important than those other hooks.  It’s also
> a
> > lot easier to comply with than trailing whitespace.
> >
> > -Ken
> >
> >
> > On 4/23/10 6:07 AM, "Berk Geveci" <berk.geveci at kitware.com> wrote:
> >
> > I am sure Brad will chime in but here is the reasoning behind not
> > having these hooks:
> >
> > Consider the case where you have a branch with 10 commits. Say the 2nd
> > one has a tab in it. When you try to push this, your whole push would
> > get rejected. Now you have to figure out how to fix the 2nd commt
> > while keeping all the commits after that the same. Note that you
> > cannot simply fix the problem by making a new commit or a revert
> > commit because the hook checks every single commit for problems. Of
> > course, fixing the 2nd commit is doable by rewriting history but it is
> > not the most straightforward thing to do.
> >
> > So we decided that commit checks would for fatal or close to fatal
> > problems and that the other problems would be caught on the dashboard
> > so that they can be fixed by later commits.
> >
> > (I disagree with the trailing space check btw. I think it is too
> > aggressive. There are many developers that use editors that are not
> > very good at not leaving trailing spaces. Forcing everyone to change
> > is to invasive)
> >
> > -berk
> >
> >
> >
> > On Fri, Apr 23, 2010 at 2:24 AM, Mark Olesen <Mark.Olesen at faurecia.com>
> > wrote:
> >> On Fri, 2010-04-23 at 10:34 +1000, Andrew Maclean wrote:
> >>> I agree with you, I  think that there also should be a commit check
> >>> for tabs,  otherwise layout gets really messy.
> >>
> >> What about checking/stripping trailing whitespace as well?
> >> This is something else that tends to look messy and result in lots of
> >> noisy commits.
> >>
> >> /mark
> >>
> >>
> >> DISCLAIMER:
> >> This electronic transmission (and any attachments thereto) is intended
> >> solely for the use of the addressee(s). It may contain confidential or
> >> legally privileged information. If you are not the intended recipient of
> >> this message, you must delete it immediately and notify the sender. Any
> >> unauthorized use or disclosure of this message is strictly prohibited.
> >> Faurecia does not guarantee the integrity of this transmission and shall
> >> therefore never be liable if the message is altered or falsified nor for
> any
> >> virus, interception or damage to your system.
> >>
> >> _______________________________________________
> >> Powered by www.kitware.com
> >>
> >> Visit other Kitware open-source projects at
> >> http://www.kitware.com/opensource/opensource.html
> >>
> >> Follow this link to subscribe/unsubscribe:
> >> http://www.vtk.org/mailman/listinfo/vtk-developers
> >>
> >>
> >
> >
> >
> >
> >    ****      Kenneth Moreland
> >     ***      Sandia National Laboratories
> > ***********
> > *** *** ***  email: kmorel at sandia.gov
> > **  ***  **  phone: (505) 844-8919
> >     ***      web:   http://www.cs.unm.edu/~kmorel
> >
> >
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://www.vtk.org/mailman/listinfo/vtk-developers
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100423/36de9b1e/attachment.html>


More information about the vtk-developers mailing list