Hi Steve, I considered this particular patch to fall in the 'trivial fix/update, clean up, and/or documentation/clarirication addition'. The posting on RB was merely an FYI.
If you are suggesting we should never push anything without first going through a review cycle, then let’s all agree to that. I personally question the cost-benefit of doing so. Andreas On 07/09/2015 15:21, "gem5-dev on behalf of Steve Reinhardt" <[email protected] on behalf of [email protected]> wrote: >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 -- 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
