How do we prevent users from merging too soon?

githubtraining / ci-circle

Course repo for Learning Lab course "Continuous Integration with CircleCI". Template repo ➡

https://github.com/githubtraining/ci-circle-template

Creative Commons Attribution 4.0 International

3 stars 9 forks source link

How do we prevent users from merging too soon? #61

Closed hollenberry closed 5 years ago

hollenberry commented 5 years ago

For PRs 1 and 2, if the user merges before adding suggested changes, this breaks the course.

Suggestion: For the first PR, can/should we add our own branch protection, and remove it before the user is instructed to do it themselves in the second PR?

For the second PR, I assume we're asking the user to "Approve the PR" to act as a webhook trigger, but I believe this may be easily misconstrued as "Merge the PR". I'm not sure the best course of action.

cc: @a-a-ron @beardofedu @brianamarie @ppremk

brianamarie commented 5 years ago

👍 @hollenberry I think it makes sense to enable branch protection until the first merge. I've opened a PR: #63

hollenberry commented 5 years ago

For the second PR, the following takes place:

I can see a user easily misinterpreting "approve" with "merge", so I'm taking the following actions in #58:

Keep the "Review required" step on master
Ask the user to edit master's rules to add a new one for CI
In the activity response, add a link to "Approve the PR", that takes the user to the approval step within the PR

brianamarie commented 5 years ago

Closing as this was (hopefully) addressed in #58 🎉