Indeed, thanks Manish! I've been assuming the only time that we would use `delegate` is for people like you (Bobby), whom we trust, but who have not gone through the process to become a Servo reviewer. Honestly, I'd rather not have this feature at all --- I would prefer that a reviewer (even if it's the PR author) at least glance at all squashes / rebases in Reviewable to verify that nothing slipped in, because we all mess up `git rebase` from time to time. But, until we have closer to 24/7 reviewer coverage, like Rust does, I was in favor of adding this support so that you are not blocked during the 1--2AM US Pacific slot that appears to be both the "deadzone" for our reviewer coverage and your prime patch-landing time :-)
I haven't been a fan of "commit" access in the past because the Servo process tends to focus on doing a lot of before-commit work (running the full test suite, doing full reviews, etc.) to avoid the need to ever close tree or back anything out. But, I can definitely see both your argument and have heard from other open source people (especially consultants and employees at BigCos) that having a "commit" level of access that's given out much more freely is a nice thing for them to be able to show to their bosses or use to market themselves, at least in the WebKit and Blink worlds. - Lars On Wed, Nov 11, 2015 at 5:47 PM, Bobby Holley <bobbyhol...@gmail.com> wrote: > Thanks for working on this Manish! > > As mentioned previously, I think that scoping this bit of trust to the PR > is sub-optimal. This kind of delegation is effectively unsupervised write > access to the repository, with an understanding that the committer is > trustworthy enough to ask for review on any significant not-yet-approved > changes. It doesn't really have anything to do with the particular PR, and > has everything to do with the person being granted that access. > > This kind of trust is a governance issue, and making it a per-reviewer > per-PR preference introduces the following problems: > * The committer needs to establish the trust relationship with each > individual reviewer, which doesn't scale for either party. > * Reviewers need to make an important trust decision for every PR, and may > not always make decisions that are consistent with what they and others > have decided previously. > * Reviewers may not always remember to set the delegate flag. > > The combination of all three of the above means that, in practice, a > contributor is likely to find themselves with carry privileges in some > situations but not others. Is that ever desirable? > > bholley > > On Wed, Nov 11, 2015 at 3:01 AM, Manish Goregaokar <manishsm...@gmail.com> > wrote: > >> After some >> <https://github.com/servo/servo/wiki/Meeting-2015-11-02#review-carry-over> >> discussion >> < >> http://logs.glob.uno/?c=mozilla%23servo&s=10+Nov+2015&e=10+Nov+2015&h=delegate#c298415 >> > >> I've landed <https://github.com/barosl/homu/pull/111> and enabled review >> delegation for homu. >> >> This means that any homu command containing `delegate=USER` will give that >> user full reviewer status for the scope of that PR (i.e., they can use all >> homu commands for that PR). You can also use `delegate+` as a shortcut to >> delegate to the PR author. `delegate-` undoes delegation. >> >> Reviewers: In case of "r=me with minor nit" situations (where the author is >> trusted enough to not add our infra to a botnet or whatever), try to use >> `@bors-servo delegate+` so that the author can finish up the PR on their >> own. This should ease up the workflow for non-reviewers. >> >> You can see it in action here <https://github.com/servo/saltfs/pull/160>. >> >> Thanks, >> -Manish Goregaokar >> _______________________________________________ >> dev-servo mailing list >> dev-servo@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-servo >> > _______________________________________________ > dev-servo mailing list > dev-servo@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-servo _______________________________________________ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
