I like the labels but dislike the +1 / -1 approach imho a reviewer can make comments on the proposed changes, some can be pure enthusiasm but most of them will be to discuss some constructs or API use that looks incorrect, and just putting such a comment with explanation why this should be reviewed is enough to make it clear some action is required, no need to add a childish :-1: When the PR looks good, "LGTM" is a clear message for a completed review by a reviewer.
2015-10-05 21:51 GMT+02:00 Oleg Nenashev <[email protected]>: > Anyway I am afraid this flame-war-in-the-making is distracting from the >> topic of the thread, which is PR labels. That seems a good idea to me. >> > > Yes, it's for another discussion. In this thread I just propose the > "ready-for-merge" status, which indicates that "there were reviewers from > Jenkins contributors/users and they have +1s on the request". Even if the > labels can be changed by jenkinsci/core team members right now, hopefully > they will help others with reviews. > > I like the labels idea. KostyaSha has been encouraging me to consider >> labels on the git plugin and git client plugin pull requests as a >> classification technique. This seems like a good generalization (in >> addition to the plugin topic specific labels that Kostya and I have >> discussed). > > > Yes, we could use the same approach in the plugins. I can teach Jenkins > IRC Bot to setup the default labels when the list gets finalized. > If nobody puts -1, I'm going to create labels tomorrow to start the test > drive > > понедельник, 5 октября 2015 г., 18:56:21 UTC+3 пользователь Mark Waite > написал: >> >> I like the labels idea. KostyaSha has been encouraging me to consider >> labels on the git plugin and git client plugin pull requests as a >> classification technique. This seems like a good generalization (in >> addition to the plugin topic specific labels that Kostya and I have >> discussed). >> >> Mark Waite >> >> On Mon, Oct 5, 2015 at 9:28 AM nicolas de loof <[email protected]> >> wrote: >> >>> Agree. I don't like all those "merge PR #" commits in project history >>> when they aren't required by some long term parallel work >>> >>> 2015-10-05 17:13 GMT+02:00 Jesse Glick <[email protected]>: >>> >>>> On Mon, Oct 5, 2015 at 8:17 AM, Tom Fennelly <[email protected]> >>>> wrote: >>>> >> The PR owner probably should not rewrite the PR history as this will >>>> lose >>>> >> review comments AFAICT. And they generally cannot merge themselves. >>>> > >>>> > That was why I said "once the review is completed". >>>> >>>> IMO the commits in the PR itself should not be squashed in any case. >>>> If the merger wishes to squash, they can commit an aggregate change to >>>> `master` with a commit message including `(closes #1234)`, leaving the >>>> PR branch with historical commits. At least that way we retain the >>>> history of development so long as we continue to host on GitHub. >>>> (AFAIK you cannot archive PR information as `git-notes` or whatever.) >>>> >>>> Anyway I am afraid this flame-war-in-the-making is distracting from >>>> the topic of the thread, which is PR labels. That seems a good idea to >>>> me. >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "Jenkins Developers" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr1F6YRSt9O35JKFLqnnvcay9-Teg3zqPpziwTKHMW5Fdg%40mail.gmail.com >>>> . >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Jenkins Developers" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/jenkinsci-dev/CANMVJznFjjcG43CaUXpT3Rfz%3D-d2S1Nc0UTgtKj1052CSa%2Bzsg%40mail.gmail.com >>> <https://groups.google.com/d/msgid/jenkinsci-dev/CANMVJznFjjcG43CaUXpT3Rfz%3D-d2S1Nc0UTgtKj1052CSa%2Bzsg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- > You received this message because you are subscribed to the Google Groups > "Jenkins Developers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/jenkinsci-dev/fc7c7aac-32dd-44b2-a558-4f25b98c3d90%40googlegroups.com > <https://groups.google.com/d/msgid/jenkinsci-dev/fc7c7aac-32dd-44b2-a558-4f25b98c3d90%40googlegroups.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Jenkins Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANMVJzmpGife1s%3DWSTWPOGkE_n02bkSP%2BmA3QKaLLX6LDcr9Gw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
