Re: Checkin-needed requests - Please include complete information in the commit message :)
On Mon, Jul 11, 2016, at 04:26 PM, Martin Thomson wrote: > On Mon, Jul 11, 2016 at 2:18 PM, Xidorn Quan wrote: > > I also use checkin-needed for small changes which I don't think it's > > worth to run a full testset for, to save some infra resources. > > Hmm, that's an odd optimization. I'd have thought that sheriff time > is more valuable than infra. So I only use it when I feel pretty confident that it wouldn't need further care and unlikely cause any conflict. Sheriff time is indeed more valuable than infra, but long backlog on infra may take sheriffs more time to handle as well, which could be more expensive than checking in a small patch. It seems to me there was still some bottleneck on our infra which could cause significant backlog when there are lots of pushes. Test machine for Windows and macOS, presumably? > On Mon, Jul 11, 2016 at 2:48 PM, Nils Ohlmeier > wrote: > > Another use case for checkin-needed are probably sec bugs, as you can’t use > > mozreview for them AFAIK. > > As for sec-critical bugs, as long as the change is going to hit the > tree with the bug number in it, then I don't see why it can't go via > mozreview. Because we don't want to reveal details before we are comfortable with disclosing them. It can go via MozReview *after* a patch is reviewed and gets sec-approval, but not the reverse. - Xidorn ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Checkin-needed requests - Please include complete information in the commit message :)
On Mon, Jul 11, 2016 at 2:18 PM, Xidorn Quan wrote: > Isn't it still necessary for people who don't yet have permission to > push? That suggests to me that there are missing safeguards on autoland. Otherwise we could just enable it even for those with try access. > I also use checkin-needed for small changes which I don't think it's > worth to run a full testset for, to save some infra resources. Hmm, that's an odd optimization. I'd have thought that sheriff time is more valuable than infra. On Mon, Jul 11, 2016 at 2:48 PM, Nils Ohlmeier wrote: > Another use case for checkin-needed are probably sec bugs, as you can’t use > mozreview for them AFAIK. As for sec-critical bugs, as long as the change is going to hit the tree with the bug number in it, then I don't see why it can't go via mozreview. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Checkin-needed requests - Please include complete information in the commit message :)
> On Jul 10, 2016, at 21:18, Xidorn Quan wrote: > > On Mon, Jul 11, 2016, at 12:29 PM, Martin Thomson wrote: >> Is now the right time to start talking about retiring checkin-needed, >> or is it still heavily used? > > Isn't it still necessary for people who don't yet have permission to > push? Another use case for checkin-needed are probably sec bugs, as you can’t use mozreview for them AFAIK. Nils signature.asc Description: Message signed with OpenPGP using GPGMail ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Checkin-needed requests - Please include complete information in the commit message :)
On Mon, Jul 11, 2016, at 12:29 PM, Martin Thomson wrote: > Is now the right time to start talking about retiring checkin-needed, > or is it still heavily used? Isn't it still necessary for people who don't yet have permission to push? I also use checkin-needed for small changes which I don't think it's worth to run a full testset for, to save some infra resources. - Xidorn ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Checkin-needed requests - Please include complete information in the commit message :)
Is now the right time to start talking about retiring checkin-needed, or is it still heavily used? On Sat, Jul 9, 2016 at 4:58 AM, Gregory Szorc wrote: > On Fri, Jul 8, 2016 at 11:39 AM, Felipe G wrote: > >> Is there a way to make the checkin-needed flag generate a template comment >> (like the approval-* ones do) with something like this? (Or encourage >> people to use the per-patch checkin? flag) >> >> """ >> Has this patch been through try? [ Yes / No, I believe it's not necessary ] >> Does this patch contain the correct author / commit message? [ Yes >> (preferred) / No, but I'm providing it here: ] >> Are there any other dependencies that should be landed together? [ Yes, ... >> / No ] >> """ >> >> Probably just asking if the information is present will reduce the number >> of requests made without it >> > > My knee jerk reaction is we shouldn't bother: MozReview handles most of > this "validation" and usage of MozReview has been steadily increasing. > We're trending towards a world where the only patches on Splinter are for > security-sensitive bugs (MozReview can't handle those yet) and the people > submitting patches to security bugs tend to know what they're doing so I > don't think these added checks will help. > > >> >> On Fri, Jul 8, 2016 at 10:47 AM, Ryan VanderMeulen >> wrote: >> >> > FWIW, there's also an MDN page that documents a lot of this as well: >> > >> > >> https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F >> > >> > -Ryan >> > >> > >> > On 7/8/2016 2:32 AM, Carsten Book wrote: >> > >> >> Hi, >> >> >> >> someone might not know that doing checkins for checkin-needed request is >> >> not automated yet and completely a fully human task :) (no we Sheriffs >> are >> >> not bots ;) >> >> >> >> It would help us a lot if a checkin needed request would contain >> complete >> >> Author/Patch information like: >> >> >> >> >> >>- Author (use the information from their Bugzilla account if needed) >> >>with Name *and *Emailadress. >> >>- Bug number >> >>- Commit message (keeping in mind that the commit message should be a >> >>brief description of what the patch is *doing*) >> >> - Format should be something like "Bug 123456 - Add a null check >> to >> >> XYZ to avoid a crash. r=somebody" >> >> >> >> >> >> And also if there is a specific sequence/dependency you want to checkin >> >> the >> >> patches it would help also a lot if you could make a short comment in >> the >> >> Bug like please checkin part x then patch y or like first bug 123 then >> >> this >> >> bug and then bug 8910. >> >> >> >> This would help us a lot :) >> >> >> >> Thanks! >> >> >> >> - Tomcat >> >> >> >> >> > ___ >> > dev-platform mailing list >> > dev-platform@lists.mozilla.org >> > https://lists.mozilla.org/listinfo/dev-platform >> > >> ___ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform >> > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
