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
