Manage GitHub settings via Probot

[dhoppe]

This pull request allows us to manage the GitHub settings via Probot. I left out the labels, as they are managed by vox-pupuli-tasks.

Maybe we can also read the file metadata.json to automatically add values for name, description, homepage and topics.

Unfortunately, the delete_merge_on_branch parameter does not work yet:

• Add support for 'auto delete PR head branch' repository-settings/app#134

Also, the rule for the modulesync branch will not be applied if it does not already exist:

• Branch protection rules don't apply for non-existent branches repository-settings/app#136

[ekohl]

I like this idea.

[ekohl]

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.

[dhoppe]

@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.

[bastelfreak]

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.

[dhoppe]

@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:

• https://github.com/dhoppe/foo/pull/1

I also was able to push to the master branch:

• https://github.com/dhoppe/foo/commit/8ef29acccdd6d1b94dfb3a59ee60e4bab96f894d

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.

[ekohl]

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

[dhoppe]

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.

[ekohl]

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?

[dhoppe]

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

[dhoppe]

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.

[ekohl]

One thing to consider is that there are people who use our modulesync on their own modules. On the other hand, they can delete the file if they want to.

[dhoppe]

We should add a section to the README.md that these two files need to be deleted via .sync.yml or modified. To be honest, we might need to convert them to .erb. This allows other projects to modify them or define excludes for a set of repositories.

[dhoppe]

I am pretty sure that this can be done easier.

[ekohl]

You can also iterate over the tuples

Suggested change

	<% branch.keys.each do |k| -%>
	<% branch.keys.each do |key, value| -%>

This avoids the whole branch[k] part.

[ekohl]

branch[k]['required_pull_request_reviews'] is nil, whether the key exists or not. It's not like Python which raises an exception if the key is not set.

[ekohl]

Not sure if delete is pretty, but I think it works:

Suggested change

	- name: <%= label['name'] %>
	<% label.keys.each do |k| -%>
	<% next if k == 'name' -%>
	<%= k %>: <%= label[k] %>
	<% end -%>
	<% end -%>
	- name: <%= label.delete('name') %>
	<% label.each do |key, value| -%>
	<%= key %>: <%= value %>
	<% end -%>
	<% end -%>

[ekohl]

Another thought for this file:

data = {}
['repository', 'labels', 'branches'].each do |key|
  data[key] = @configs[key] if @configs[key]
end
-%>
<%= data.to_yaml %>

Edit: even shorter:

<%= @configs.slice('repository', 'labels', 'branches').to_yaml %>

[dhoppe]

That is what I was looking for, but it was so late. However ~ is not interpreted as null.

[ekohl]

Untested, but I think this will result in the same:

Suggested change

	<% if @configs['permissions'] -%>
	<% @configs['permissions'].each do |key, value| -%>
	<%= key %> <%= value %>
	<% end -%>
	<% end -%>
	<% @configs['permissions']&.each do |key, value| -%>
	<%= key %> <%= value %>
	<% end -%>

[dhoppe]

It does. Honestly, how many Ruby books do you have hidden under your pillow? ;)

[ekohl]

None, I still consider myself a novice Ruby developer tbh. Just look at a bunch of other devs and PR reviews. Copy that. That's how I learned the language constructs. This one is called the safe navigation operator and was introduced in Ruby 2.3.

Also read the core data structures (Array, Hash, String and Integer) when you deal with them. The list of methods is useful.

Once you have a decent understanding of what's possible, you can express yourself much more powerful.

[dhoppe]

I think now we have a working version that we could test on a repository.

This should cover most of the features of voxpupuli/vox-pupuli-tasks#14

[bastelfreak]

this will send an email to every collaborator, right? Is that maybe too noisy?

[ekohl]

Right now I purposely unwatched a lot of repositories precisely because of it.

[dhoppe]

Damn. This was supposed to be a failsafe, according to the docs of Probot. I will drop the last line, but the first one is necessary:

• https://github.com/probot/settings#security-implications

[dhoppe]

I think I understand the problem now. GitHub allows things on its website that do not work with Curl or Probot.

I actually think it is quite nice when a review of the pull request is needed. However, this does not work with the task travis_release. Therefore I made two changes.

However, I will still contact GitHub support to have branches automatically deleted after a merge and to create protection rules for non-existent branches.

[wyardley]

Maybe out of scope for this specific PR, but has there been discussion of changing the default branch name to main?

[ekohl]

AFAIK there has not been any discussion on this.