I don't have any problem with the particular patch itself, which is why I didn't complain about it specifically. I was a bit thrown off by the non-standard process though. It was the effort required to determine that this patch had indeed followed a non-standard process (and it not being the first time I've seen a commit fly by and thought "did that really get adequately reviewed?") that kicked off this whole thread...
Steve On Sun, Sep 6, 2015 at 8:03 AM Andreas Hansson <[email protected]> wrote: > Hi Steve, > > Do you have any concern with the patch? Given that it is a rather obvious > fix (and a one-liner), I decided not to wait. > > Andreas > > On 04/09/2015 18:54, "gem5-dev on behalf of Steve Reinhardt" > <[email protected] on behalf of [email protected]> wrote: > > >To be concrete, Andreas, I saw your commit of the "Avoid setting > >markPending" changeset, and I thought "gee, I wonder how long that was on > >reviewboard, and whether it got any ship-its, because I don't recall > >reviewing it". I was able to find it by searching my email for > >"markPending" to see the review request (and to find out that it had only > >been posted for just over four days and had not received any ship-its, or > >any reviews at all---but that's a separate issue). It would have been much > >easier to answer my initial question if there had been a link in your > >commit message, and I can see the link being even more valuable when a > >long > >time has passed since the review request was posted, or if the description > >doesn't contain any particularly unique terms to simplify a search. > > > >To answer Jason's question: it would be nice to have an automation system > >where someone with sufficient permissions could just click on a patch on > >reviewboard and the patch would automatically be committed; with that > >process in place, it doesn't seem like much of a stretch to include the > >URL > >in the commit message. I know Ali has discussed some other tools that > >enable this. The easiest path to this kind of functionality might require > >migrating to something other than reviewboard. > > > >Steve > > > > > >On Fri, Sep 4, 2015 at 10:38 AM Andreas Hansson <[email protected]> > >wrote: > > > >> No strong opinion in either direction. I have not experienced a > >>situation > >> where it was needed, but I also do not think it is too much to ask. > >> > >> Andreas > >> > >> On 04/09/2015 18:34, "gem5-dev on behalf of Gutierrez, Anthony" > >> <[email protected] on behalf of [email protected]> > >>wrote: > >> > >> >I like that idea. Or at least the review number, for brevity's sake. > >> > > >> >-----Original Message----- > >> >From: gem5-dev [mailto:[email protected]] On Behalf Of Steve > >> >Reinhardt > >> >Sent: Friday, September 04, 2015 10:32 AM > >> >To: gem5 Developer List > >> >Subject: [gem5-dev] including reviewboard URL in commit message? > >> > > >> >Hi everyone, > >> > > >> >Just a thought: should we require commit messages to include the URL of > >> >the corresponding reviewboard posting? I think that would be useful in > >> >tying the discussion to the commit. Apologies if someone suggested > >>this > >> >before and I missed it. > >> > > >> >Steve > >> >_______________________________________________ > >> >gem5-dev mailing list > >> >[email protected] > >> >http://m5sim.org/mailman/listinfo/gem5-dev > >> >_______________________________________________ > >> >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. > >> > >> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > >> Registered in England & Wales, Company No: 2557590 > >> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 > >>9NJ, > >> Registered in England & Wales, Company No: 2548782 > >> _______________________________________________ > >> gem5-dev mailing list > >> [email protected] > >> http://m5sim.org/mailman/listinfo/gem5-dev > >> > >_______________________________________________ > >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. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2557590 > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2548782 > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
