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

Proposal: Check the branches that PRs are raised against and from #17

Open
rgee0 opened this issue Nov 4, 2017 · 8 comments
Open

Proposal: Check the branches that PRs are raised against and from #17

rgee0 opened this issue Nov 4, 2017 · 8 comments

Comments

@rgee0
Copy link
Contributor

rgee0 commented Nov 4, 2017

Upon submission of a PR Derek should check the originating branch name, and if he finds master the PR closed with a message advising the user that they should re-submit from a non-master branch.

Also, check that pull request is against master / default.

@alexellis
Copy link
Owner

Thank you for logging this

@rgee0 rgee0 changed the title Proposal: Close PRs that are raised master to master Proposal: Check the branches that PRs are raised against and from Nov 7, 2017
@alexellis
Copy link
Owner

Still a fan of this feature

@alexellis
Copy link
Owner

Derek add label: priority/low

@alexellis
Copy link
Owner

alexellis commented Feb 25, 2018

The idea is here that:

PRs should only be raised to target master - therefore if someone raises a PR that doesn't - apply a label.

I.e.

I raise a PR to a branch named 0.7.0 - that should add a label, I don't know what label but am open to suggestions: attention/target-branch.

2nd scenario:

Someone raises a PR from the master branch of their repo. This can be destructive to merge - especially if that person has been rebasing commits / force pushing or even altering historic commits.

Derek should add a warning comment:

It appears that you are submitting changes directly from your master branch, we encourage you to raise a new pull request from a named branch i.e. git checkout -b my_feature.

It should also add a label like: attention/source-branch

In both scenarios the user can only raise a new PR to fix the problem. This means no code is needed to remove the labels.

@johnharris85
Copy link

So adding a label rather than closing the PR? Would this be a separate 'feature' that could be enabled / disabled in the config yaml? Would these 2 scenarios (target and origin) be separate features? Or part of a single 'branch check' feature?

@alexellis
Copy link
Owner

This would be worth doing - even at a simplistic level - no PRs from master for instance.

@ivanayov
Copy link
Contributor

Taking this

@alexellis
Copy link
Owner

I met with Ivana and Dimitar. We think the action should be:

  • Close the PR (because you can't recover without creating a new PR from a different branch)
  • Give a comment - should say a git command to help them such as git checkout -b my_feature and raise a PR from there
  • Ivana liked the idea of the label - let's use the prefix of "review/" to match the listing in [Merge/release] Add commit message linting feature #43

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

No branches or pull requests

4 participants