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

Require pull request before merging #20

Closed
Tracked by #48 ...
nweldev opened this issue Apr 27, 2022 · 7 comments
Closed
Tracked by #48 ...

Require pull request before merging #20

nweldev opened this issue Apr 27, 2022 · 7 comments
Assignees
Labels
effort1: easy (hours) priority: 3 (necessary) Medium priority issue that needs to be resolved scope: tooling
Milestone

Comments

@nweldev
Copy link

nweldev commented Apr 27, 2022

What?

Add a branch protection rule on all Github repositories to forbid anyone except two "repository managers" to make any update in the main branch without describing/discussing it first.

Why?

Pushing changes directly to the main branch makes it difficult to track and discuss them, with is a clear infringement of our openness framework, and can discourage contributions.

Example: https://github.com/blindnet-io/clients-management/issues/11#issuecomment-1111016494

How?

GitHub branch protection rules are only available for public repositories and only to Pro, Team, and Enterprise users on private repositories. However, this isn't a big issue as most of our repositories (especially code ones) will soon be public. Restricted management repositories don't require such restrictions, as their code is pretty simple and not subject to a lot of changes.

see GitHub documentation: Managing a branch protection rule

@filipblnt
Copy link
Member

Note: the same rule should apply to develop branch as well.

@nweldev
Copy link
Author

nweldev commented May 15, 2022

I believe this should be linked to a Definition of Done for development (i.e. requirements to meet before merging a PR and closing the related issue), including links/explanations explaining why/how an issue was closed, peer-review, and PR description (see #5).

@nweldev nweldev added priority: 1 (urgent) Urgent priority issue that should be resolved before the next release/phase and removed scope: community Community Building and Management labels May 18, 2022
@nweldev nweldev added OF Triage: open priority: 3 (necessary) Medium priority issue that needs to be resolved and removed priority: 1 (urgent) Urgent priority issue that should be resolved before the next release/phase labels Jun 2, 2022
@nweldev nweldev transferred this issue from another repository Jun 3, 2022
@nweldev nweldev transferred this issue from another repository Jun 3, 2022
@nweldev nweldev added this to the Q3 2022 milestone Jun 6, 2022
@nweldev nweldev moved this to In Progress in Technical Communications Jul 27, 2022
@nweldev
Copy link
Author

nweldev commented Jul 27, 2022

Branch protection rules were added for the main and develop branches to :

  • require pull request
  • require at least one approval
  • Dismiss stale pull request approvals when new commits are pushed
  • require conversation resolution

before merging on:

This would be useless on private repositories as the "protected branch rules won't be enforced on this private repository until you upgrade your organisation account to a GitHub Team or Enterprise account".

@nweldev
Copy link
Author

nweldev commented Jul 27, 2022

Added a less restrictive rule for docs to just require a pull request (without absolute review requirement) before merging to main on:

Also added the more restrive rules from the above comment, but only of the main branch of https://github.com/blindnet-io/openness-framework

@nweldev
Copy link
Author

nweldev commented Jul 27, 2022

@blindnet-io/engineering Please remember to add same rules to all new DevKit source code repositories between v0.4 and v0.6

@nweldev nweldev assigned TheKinrar, jboileau99 and m4rk055 and unassigned nweldev Jul 27, 2022
@nweldev nweldev moved this from In Progress to On Hold in Technical Communications Jul 27, 2022
@nweldev
Copy link
Author

nweldev commented Aug 13, 2022

All actively maintained repositories should now have the required branch protections.

If you find a repository for which these protections haven't been configured, please directly add them and add a comment below if you have the required rights, or reopen this issue and tell us which repository should be configured.

@nweldev nweldev closed this as completed Aug 13, 2022
Repository owner moved this from 🛑 On Hold to ✅ Done in Technical Communications Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort1: easy (hours) priority: 3 (necessary) Medium priority issue that needs to be resolved scope: tooling
Projects
Status: Done
Development

No branches or pull requests

5 participants