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

Enable developers to merge code to *-dev branch without branch protection #26

Open
praveensameneni opened this issue Aug 9, 2021 · 6 comments

Comments

@praveensameneni
Copy link
Member

We have branch protection for merging code to a dev branch. Can we remove branch protection on -dev branch where an "" can represent the name of a feature - e.g bucket-level-alerting-dev

Apply branch protection rules only to
main
opensearch-[1-9].[0-9].[0-9]*

@dblock
Copy link
Member

dblock commented Aug 9, 2021

Is this for the OpenSearch repo? I'll move this issue to that repo then. There's no standard branch protection for anything other than main org-wide.

@qreshi
Copy link

qreshi commented Aug 10, 2021

@dblock This was encountered for the Alerting OpenSearch repo. We don't have insight into the branch protection rules in the Settings but after I pushed the dev branch bucket-level-alerting-dev into the repo, it prevented any subsequent commits because the branch was marked as "protected" (so perhaps all branches are protected org-wide since we don't have permissions to change the repo specific settings). I think feature branches should be excluded from branch protection (or at least the PR requirement) to allow for fast development and then ensure they are PRd when merging into main (if they have not been already).

For the time being, we'd at least like the bucket-level-alerting-dev branches in the OpenSearch Alerting and OpenSearch Alerting Dashboards to allow for commits without PRs since it's impeding on development velocity.

@dblock
Copy link
Member

dblock commented Aug 10, 2021

@dblock This was encountered for the Alerting OpenSearch repo. We don't have insight into the branch protection rules in the Settings but after I pushed the dev branch bucket-level-alerting-dev into the repo, it prevented any subsequent commits because the branch was marked as "protected" (so perhaps all branches are protected org-wide since we don't have permissions to change the repo specific settings). I think feature branches should be excluded from branch protection (or at least the PR requirement) to allow for fast development and then ensure they are PRd when merging into main (if they have not been already).

For the time being, we'd at least like the bucket-level-alerting-dev branches in the OpenSearch Alerting and OpenSearch Alerting Dashboards to allow for commits without PRs since it's impeding on development velocity.

Makes sense. Right now org-wide we don't have any rules related to any other branches than main and release branches such as 1.0 or 1.x. We actually don't spell out branch protection, but we should somewhere. So I suggest you start by PRing an update to https://github.com/opensearch-project/.github/blob/main/RELEASING.md#branching (and possibly ADMINS.md) that describes in generic terms what you'd like to happen in terms of branch protection rules? And we can discuss and agree? If it gets merged then it becomes an org wide thing.

@CEHENKLE
Copy link
Member

seconded dB's comment. Let's get concrete on the proposal, and push it across the repos when we move.

@dblock
Copy link
Member

dblock commented Aug 11, 2021

it prevented any subsequent commits because the branch was marked as "protected"

@qreshi Branch protection just says that you need to be doing PRs into the branch and not push directly to avoid accidentally overwriting someone else's code, and disallow force pushes, and that PRs need to be reviewed by at least 1 person. It does not prevent you from subsequent commits!

I think it's good that ya'll can't push into alerting/dev, but have to make a PR (small price to pay), then if you don't like that 1 other person needs to code review for dev branches, just agree that the "code review" is an ack by another engineer. It doesn't happen often.

I am happy to go edit branch protections for alerting for whatever though, such as changing * to main, but just consider my comment above as a safety measure.

@peternied
Copy link
Member

Alternative - Fork repositories for development scenarios

Can we use fork branches for these kinds of development scenarios? Multiple contributors can work within a fork on a feature, still using PRs, and then when the work is ready it can follow the standard pull request pattern to rejoin its the target branch in the opensearch-project repository.

Forks can use GHA just like the opensearch-project based repository, or additional requirements if need be. This allows customization to cater to the work under development.

Some things that will not work are workflows from the Jenkins system, those are locked into opensearch-project based repository, so gradle check like tasks might need to be considered if they cannot run in GHA


This effectively side-steps the branch protection policy - does that work?

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

No branches or pull requests

5 participants