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 branches to be up to date before merging #1085

Closed
atugushev opened this issue Mar 4, 2020 · 8 comments
Closed

Require branches to be up to date before merging #1085

atugushev opened this issue Mar 4, 2020 · 8 comments
Assignees
Labels
maintenance Related to maintenance processes

Comments

@atugushev
Copy link
Member

What's the problem this feature will solve?

Currently, the master might be broken by merging a PR that has not been tested with the latest code. I believe we could adjust the repository settings to avoid this issue.

Describe the solution you'd like

Update the repository settings to protect the master branch:

image

Additional context

#824 (comment)

@atugushev
Copy link
Member Author

kindly ping @jezdez

@jezdez
Copy link
Member

jezdez commented Apr 24, 2020

@atugushev which of the status check should be required to be passing (as mentioned in the help text to the "Require branches to be up to date before merging")?

Status checks found in the last week for this repository:
codecov/patch
codecov/project
continuous-integration/appveyor/branch
continuous-integration/appveyor/pr
continuous-integration/travis-ci
coverage/coveralls
test (2.7, pipmaster, ubuntu-latest)
test (2.7, pipmaster, windows-latest)
test (3.5, pipmaster, ubuntu-latest)
test (3.5, pipmaster, windows-latest)
test (3.6, pipmaster, ubuntu-latest)
test (3.6, pipmaster, windows-latest)
test (3.7, pipmaster, ubuntu-latest)
test (3.7, pipmaster, windows-latest)
test (3.8, pipmaster, ubuntu-latest)
test (3.8, pipmaster, windows-latest)

@atugushev
Copy link
Member Author

@jezdez I wouldn't require any of that, because the coverage could be decreased or tests failed (canceled for example) for any unimportant reason and it could be inappropriate to block merge on that.

@jezdez
Copy link
Member

jezdez commented Apr 24, 2020

The feature requires enabling at least one:

Screen-Shot-2020-04-24-20-11-17

@atugushev
Copy link
Member Author

Okay... Let's try these two then:

  • continuous-integration/appveyor/pr
  • continuous-integration/travis-ci

@altendky
Copy link

For GitHub Actions I add an all job that depends on the others so I can select a single item for the GitHub actions builds. This way when you change matrixing, or even just job names, you don't have to update your GitHub branch policies. Also, depending on other selections admins can override these conditions on the PR page when merging. But yeah, if GitHub Actions isn't primary then this is a side-note FYI.

https:/pytest-dev/pytest-twisted/blob/7cf213bacf37240995b8d6bb46cf90935b05f2fd/.github/workflows/ci.yml#L120-L130

  all:
    name: All
    runs-on: ubuntu-latest
    needs:
      - test
      - linting
    steps:
      - name: This
        shell: python
        run: |
          import this

@jezdez
Copy link
Member

jezdez commented Apr 24, 2020

@atugushev Done.

@jezdez jezdez closed this as completed Apr 24, 2020
@atugushev
Copy link
Member Author

@altendky thanks for the trick! GitHub Actions CI is on the way.

@jezdez thank you!

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

No branches or pull requests

3 participants