Hi Andreas, Thanks for the clarification. Point taken for "Reported-by" :).
Jason On Thu, May 12, 2016 at 9:50 AM Andreas Sandberg <[email protected]> wrote: > Hi Jason, > > At least git keeps track of both the patch author and who pushes a patch. > The signed-off-by chain would also fill this function. These tags are > typically used (some organisations may enforce stricter rules locally) as > statement by the developer saying “this change is technically sound and > I’m allowed to commit it”. Similarly, whoever pushes the patch using > today’s system would sign off the patch since they are effectively > forwarding it through their tree. > > As for checking line breaks, I don’t really know if there is a good tool > for that. Gerrit definitely shows a read line at column 75 when reviewing > patches. Emacs seems to do “the right thing” when pressing M-q. Don’t know > about VI though. > > We could/should also write a commit message hook that enforces formatting > similar to how we enforce code style. I’m tempted to offer my services > here, but I currently have a lot of other projects going and I’m not sure > if I could follow up on that commitment within a reasonable time frame. > > I’m not sure I agree with you that we need a bug tracking system for > "Reported-by” to be useful. It would be useful for cases where someone on > one of the mailinglists points out an issue and someone else fixes it. > > Cheers, > Andreas > > On 12/05/2016, 15:18, "gem5-dev on behalf of Jason Lowe-Power" > <[email protected] on behalf of [email protected]> wrote: > > >Hi Andreas, > > > >The updates to the character limits and formatting changes sound good to > >me. Is there any way to make it easy to verify my commit messages meet > >these requirements? My current approach is "ehhh, this looks like 70 > >characters or so...". Maybe git has a good way to do this? > > > >As far as the tags go, "Reviewed by" sounds great to me. > > > >I'm not sure about the "Signed-off-by", though. Currently we don't have > >any > >process for who is required to sign off on a patch before it is committed. > >Though, I think this was proposed in this preliminary governance document > >we never actually "ratified": > > > https://docs.google.com/document/d/12X3YHGJShUS34sorQJIvcr5NUssZcmMIJovD9I > >3w0Ns/edit?usp=sharing > >Similarly, we don't really have a method to report bugs either, so I'm not > >sure if "Reported-by" is needed, unless we implement (read: *use*) a > >reporting system. > > > >Maybe we also need a "Pushed-by" or is this the same thing as > >"Signed-off-by"? > > > >Thanks for bringing this up! > > > >Jason > > > >On Thu, May 12, 2016 at 9:08 AM Andreas Sandberg > ><[email protected]> > >wrote: > > > >> I just realised that I forgot a useful tag, we should probably add a > >> “Reported-by:” to acknowledge people who report bugs. > >> > >> //Andreas > >> > >> On 12/05/2016, 14:37, "gem5-dev on behalf of Andreas Sandberg" > >> <[email protected] on behalf of [email protected]> > wrote: > >> > >> >Hi Everyone, > >> > > >> >I¹d like to open up a discussion around the gem5 commit message style > >> >guide. In particular, I¹d like to make life better for users of > >>git-baesd > >> >tools, improve tracking, and acknowledge reviewers. > >> > > >> >The current style guide[1] mandates this: > >> > > >> > * The first line of a commit message is a summary line > >> > * Subsequent lines provide a detailed description > >> > > >> > * The summary line starts with a series of keywords separated by > >>commas > >> >followed by a colon and a short patch description. > >> > * The summary line can¹t be longer than 65 characters. > >> > > >> > * The detailed description is wrapped to fit in 80 columns. > >> > > >> > > >> >I would propose to make three changes: > >> > > >> >1. Limit detailed lines to 75 characters > >> > > >> >A lot of tools (especially git-based tools) nowadays seem to assume > >>that > >> >the detailed description is wrapped at 75 characters or less. The Git > >>Book > >> >and GitHub suggest ³72 characters or so² [2, 4], Linux mandates ³70-75 > >> >characters² [3]. I¹d suggest that that we limit summary lines to 75 > >> >characters. This is in practice the upper limit imposed by the kernel > >>and > >> >would work nicely with the way git indents log lines. > >> > > >> >2. Add a blank line between the summary line and the detailed > >>description > >> > > >> >This seems to be suggested by Linux, GitHub and the Git Book. > >> > > >> > > >> >3. Additional tracking and acknowledgements > >> > > >> >A long-term change that I¹d like to see is adding more tracking to > >>commit > >> >messages. In particular, I would like to introduce these tags (see [3] > >>for > >> >a description of how they are used in Linux): > >> > > >> > * Reviewed-by: This would be used to acknowledge the work of > >>reviewers. > >> >We already use this internally and that have gone through internal > >>review > >> >within ARM will have these tags. > >> > > >> > * Signed-off-by: This tag tells other developers that the patch author > >> >has the right to submit this work. Similarly, people in the delivery > >>path > >> >of a path add a this tag to say to indicate that they have received it > >>in > >> >good faith and forwarded it. See [5] for a description of how the > >>kernel > >> >uses it. Any modifications (rebasing) would be described within > >>brackets. > >> >For example: > >> > > >> > Signed-off-by: Foo Bar <. . .> > >> > [Rebased to work with latest gem5] > >> > Signed-off-by: Andreas Sandberg <. . .> > >> > > >> > > >> >Another good example would be the fork patches [6]. These were written > >>by > >> >me before I joined ARM, rebased by Sascha Bischoff, and finally pushed > >>by > >> >me. > >> > > >> >Cheers, > >> >Andreas > >> > > >> >[1] http://www.gem5.org/Submitting_Contributions > >> > > >> >[2] https://git-scm.com/book/ch5-2.html > >> > > >> >[3] > >> > >>> > http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L60 > >>>0 > >> > > >> >[4] https://github.com/blog/926-shiny-new-commit-styles > >> > > >> >[5] > >> > >>> > http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L40 > >>>9 > >> > > >> >[6] > >> > > >> > >> > https://github.com/gem5/gem5/commit/738d71f6a93e2da9ef5bb1490f2b94f954173 > >>5 > >> >e > >> >3 > >> > > >> >IMPORTANT NOTICE: The contents of this email and any attachments are > >> >confidential and may also be privileged. If you are not the intended > >> >recipient, please notify the sender immediately and do not disclose the > >> >contents to any other person, use it for any purpose, or store or copy > >> >the information in any medium. Thank you. > >> > > >> >_______________________________________________ > >> >gem5-dev mailing list > >> >[email protected] > >> >http://m5sim.org/mailman/listinfo/gem5-dev > >> > >> IMPORTANT NOTICE: The contents of this email and any attachments are > >> confidential and may also be privileged. If you are not the intended > >> recipient, please notify the sender immediately and do not disclose the > >> contents to any other person, use it for any purpose, or store or copy > >>the > >> information in any medium. Thank you. > >> _______________________________________________ > >> gem5-dev mailing list > >> [email protected] > >> http://m5sim.org/mailman/listinfo/gem5-dev > >> > >-- > > > >Jason > >_______________________________________________ > >gem5-dev mailing list > >[email protected] > >http://m5sim.org/mailman/listinfo/gem5-dev > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > -- Jason _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
