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

Wait a bit more before requiring approving reviews #53

Closed
choldgraf opened this issue Feb 18, 2020 · 4 comments
Closed

Wait a bit more before requiring approving reviews #53

choldgraf opened this issue Feb 18, 2020 · 4 comments
Labels
discussion no fixed close condition
Milestone

Comments

@choldgraf
Copy link
Member

My feeling is that this repository is still evolving quite rapidly (eg the documentation). Can we hold off on requiring an approving review until we think the repo is in a more steady state, and then turn it on?

Also, we should discuss as a group whether we want to impose these kinds of restrictions on the repositories in the EBP org (eg a requirement that a person reviews the PR before it may be merged). It may be the right decision, but either way it should be a group decision.

@chrisjsewell
Copy link
Member

Its probably me being a control freak, but I do feel its best to have a second pair of eyes at least take a cursory glance at PRs to sanity check them.
For example in #50 I just wanted to check you were happy with the added documentation, rather than you going it and re-editing it later.

@chrisjsewell chrisjsewell added the discussion no fixed close condition label Feb 19, 2020
@chrisjsewell
Copy link
Member

I'll turn it off for now, but would like to re-instate it asap

@choldgraf
Copy link
Member Author

I think it makes sense to adopt this behavior after we've moved past the initial "rapid prototyping and building stage" of myst. I think we are close to that point but there may still be some big-ish changes that will benefit from the fluidity of self-merging.

Once we think this is, say, "alpha 0.1" ready, then I agree we should either require PRs to have a review or simply adopt team practices that nobody is allowed to merge their own PR unless it is exceptional. (I have found it may be useful to make it more of a team practice than a github restriction, in the rare case that it is helpful to self merge, such as if there is a critical bug you want fixed right away)

@chrisjsewell
Copy link
Member

Now is that time 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion no fixed close condition
Projects
None yet
Development

No branches or pull requests

2 participants