mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Artem Harutyunyan" <ar...@mesosphere.io>
Subject Re: Review Request 41586: Partially enforced commit message guidelines with a hook.
Date Sat, 26 Dec 2015 02:05:22 GMT


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > bootstrap, lines 22-23
> > <https://reviews.apache.org/r/41586/diff/1/?file=1172700#file1172700line22>
> >
> >     Our general policy in C++ variable names is to avoid abbreviations, and I imagine
that could apply to shell scripts too.
> >     Please s/msg/message/

That's a standard git hook file name. https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, line 3
> > <https://reviews.apache.org/r/41586/diff/1/?file=1172701#file1172701line3>
> >
> >     "A hook script to verify commit message format. Called by..." so that the first
sentence explains what the script does, rather than how it's called.

Actually that comment came with the template hook file (.git/hooks/commit-msg.sample).


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, line 4
> > <https://reviews.apache.org/r/41586/diff/1/?file=1172701#file1172701line4>
> >
> >     "should"? Are you unsure if it will?

I believe that the `if it wants to stop the commit` part of the sentence makes it pretty explicit,
no? Again, this comment is part of the template hook file. If you have a suggestion on how
to rewrite it I'll gladly follow it :-).


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, lines 6-7
> > <https://reviews.apache.org/r/41586/diff/1/?file=1172701#file1172701line6>
> >
> >     # To enable this hook, do this from the root of the repo:
> >     #
> >     # $ ln -s ../../support/hooks/commit-message .git/hooks/commit-message

In our case it's done automatically by the bootstrap script. It's part of this same commit.
Added a clarifying comment.


> On Dec. 24, 2015, 2:27 a.m., Adam B wrote:
> > support/hooks/commit-msg, line 19
> > <https://reviews.apache.org/r/41586/diff/1/?file=1172701#file1172701line19>
> >
> >     None of the lines should exceed 72 chars, and the first line should ideally
be even less (~50). Ideally this hook would error for a too-long summary line, and wrap the
description.

Makes sense. Added a JIRA https://issues.apache.org/jira/browse/MESOS-4250 to follow up.


- Artem


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41586/#review111839
-----------------------------------------------------------


On Dec. 25, 2015, 6:05 p.m., Artem Harutyunyan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41586/
> -----------------------------------------------------------
> 
> (Updated Dec. 25, 2015, 6:05 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Partially enforced commit message guidelines with a hook.
> 
> 
> Diffs
> -----
> 
>   bootstrap 89d986fd95dc16bbb79623ef92e3b14a2e7009f9 
>   support/hooks/commit-msg PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41586/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message