Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loosen FCP restrictions #188

Merged
merged 2 commits into from Feb 16, 2018

Conversation

Projects
None yet
4 participants
@aturon
Copy link
Member

aturon commented Feb 16, 2018

Allows an item to enter FCP when a majority of team members has approved
and no team member has objected. This retains the consensus process,
but makes it so that FCP isn't blocked by default. Rather, members who
object to the proposal need to register a concern to block it.

@aturon

This comment has been minimized.

Copy link
Member Author

aturon commented Feb 16, 2018

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Feb 16, 2018

Hmm, I'm torn. I agree that the current model is annoying. I'm not sure that majority is sufficient "quorum". I know that I often don't register an objection but also don't click because I want to think -- probably it's good to force me to articulate my concerns though. So 👍 on the idea, probably 👍 on the specifics.

@aturon aturon force-pushed the aturon:lazy-consensus branch from e36aa13 to 7a55d7f Feb 16, 2018

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Feb 16, 2018

I just realized that all this does is ENTER the FCP period. I was concerned about things going "too fast", but that 10 day period is enough time. I say :shipit: !

@aturon

This comment has been minimized.

Copy link
Member Author

aturon commented Feb 16, 2018

@anp Please hold off on merging until I've heard from the rest of the core team on this -- I'll let you know,

@anp

anp approved these changes Feb 16, 2018

@@ -224,6 +224,7 @@ fn evaluate_nags() -> DashResult<()> {
};

let num_active_reviews = reviews.iter().filter(|&&(_, ref r)| !r.reviewed).count();

This comment has been minimized.

@anp

anp Feb 16, 2018

Member

Maybe this should be renamed to num_outstanding_reviews or num_pending_reviews to contrast it with complete reviews?

@@ -255,7 +256,7 @@ fn evaluate_nags() -> DashResult<()> {
};
}

if num_active_reviews == 0 && num_active_concerns == 0 {
if (num_active_reviews < num_complete_reviews) && num_active_concerns == 0 {

This comment has been minimized.

@anp

anp Feb 16, 2018

Member

We should obviously do whatever works best for the core team, but there are other options here than "all reviewed" or "majority reviewed" like 2/3, all but 1 (to account for people being away), etc.

Loosen FCP restrictions
Allows an item to enter FCP when a majority of team members has approved
and *no* team member has objected. This retains the consensus process,
but makes it so that FCP isn't blocked *by default*. Rather, members who
object to the proposal need to register a concern to block it.

@aturon aturon force-pushed the aturon:lazy-consensus branch from 7a55d7f to b7b4e3f Feb 16, 2018

@aturon

This comment has been minimized.

Copy link
Member Author

aturon commented Feb 16, 2018

Updated per @anp's review, and with the logic that there must be less than 3 reviews outstanding (and a majority must have approved).

I think we're good to go on this!

@anp anp merged commit 44e453e into rust-lang:master Feb 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.