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

Manage GitHub settings via Probot #654

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions moduleroot/.github/settings.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
repository:
private: false
has_issues: true
has_projects: false
has_wiki: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, but have we checked whether this is actually true? Or do you want to do that as part of the modulesync review?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to loop over managed_modules.yml to fetch the current values regarding projects and wiki.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many repositories use projectsand / or wikibecause this is a default value during the creation process. I think we need to check this manually during the modulesync and modify it if necessary via .sync.yml.

has_downloads: true
default_branch: master
allow_squash_merge: true
allow_merge_commit: true
allow_rebase_merge: true
delete_branch_on_merge: true
archived: false

branches:
- name: master
protection:
required_pull_request_reviews:
required_approving_review_count: 1
dismiss_stale_reviews: true
require_code_owner_reviews: true
required_status_checks:
strict: true
contexts: ['continuous-integration/travis-ci']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn't work for our release workflow. Also not sure if there might be a difference between legacy open source Travis and the modern one. Might be that we have some modules using the modern one.

Copy link
Member Author

@dhoppe dhoppe May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastelfreak mentioned that the travis_relase task might fail, but we could set enforce_admins: null to override this behavior.

I do not know, if we are able to check the current Travis CI version, but it should be possible to configure multiple contexts like Travis CI and Jenkins. In the end it is an array, but I have to take a look at the docs, if this will be treated as and condition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as far as I know this will break our release process (that's why I tested this long time ago and had to remove the branch protection again). This will enforce that travis runs for every PR and only PRs are allowed for the protected branch. You cannot push anymore directly to master. This in general should be our goal, but our travis_release task directly pushes to master:
https://github.com/voxpupuli/voxpupuli-release-gem/blob/fb73087fda1f64b79ee3bb668e3a4ce6cd8626b1/lib/voxpupuli/release/rake_tasks.rb#L32-L33

If I remember corretly nobody can push to master, even admins. admins "only" have the power to merge PRs if travis is still running/failing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastelfreak I think you have missed my comment regarding enforce_admins: null.

I was able to merge a pull request, although the conditions for the Travis CI check were not fulfilled:

I also was able to push to the master branch:

This might be related to the CODEOWNERS.

I think it would be best if I make a template from the files and we start with some small changes in the config_defaults.yml file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has to be tested by Travis, but Travis/Github identifies by commit. That means you can push it to a branch, get it tested and merged. Perhaps you can change the script to push to a next-release branch and set up auto merge rules for that with something like mergify

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove the required_pull_request_reviews, required_status_checks, enforce_admins and restrictions to get started.

Thanks for the tip on mergify. I will ll take a look at it later.

enforce_admins: true
restrictions: null
required_signatures: true
- name: modulesync
protection:
required_pull_request_reviews: null
required_status_checks:
strict: true
contexts: ['continuous-integration/travis-ci']
enforce_admins: true
restrictions: null
required_signatures: true

3 changes: 3 additions & 0 deletions moduleroot/CODEOWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.github/settings.yml @voxpupuli/project-maintainers

* @voxpupuli/collaborators