Proposal to protect master branch to enforce creating Pull Requests for changes

[mguihal]

Problem It can sometimes be difficult to keep track of the various improvements made to the project for the different developers and maintainers. I noticed that some improvements were made via PRs, while others were committed directly to the master branch. This makes it all the more difficult to keep track of changes.

Proposal I'm proposing to set up protection for the master branch to force the creation of pull requests as soon as a code change is proposed.

Why is opening a PR better?

• It allows other developers and maintainers on the project to be aware of proposed improvements.
• It gives the context of why the change is being made (the need, the bug, etc.).
• It allows discussion around the change, to question it, or make it better, or detect a not-seen breaking change
• Better code democracy!

Why would opening a PR be any worse?

• It slows down development (but makes it more explicit)
• It is possible to open a PR and merge it immediately, which may partially defeat its purpose.
• Some master commits are still necessary (e.g. version commits).

What do you think about it?

Alternatives Maybe you have other proposals to make it better to follow code changes and discuss about it?

[srosset81]

Sorry, but being the main SemApps contributor at the moment, and doing most of the fixes/improvements for free, I don't always have the patience to create PRs. The other day I did 5 little fixes and creating 5 different PRs would have taken me twice more time. I'm documenting every commits in the releases (it's already a lot of work), I think it already helps to follow what's happening.

Apart from this personal problem, my opinion is that for very little one-line fixes, creating a PR is an overkill. It would also be a problem for version commits, as you mention, forcing us to do a PR for every version change (and right now the yarn run version command can only be run in the master or next branches).

I will try to do more PRs, especially when it's not one-line fixes, but preventing direct commits on the master branch is IMO too much of an hassle in the current state of this project.

PS: Feel free to enforce this on Archipelago

[mguihal]

That's ok. I opened this issue mainly to warning you about the difficulty to follow and understand changes on this project, without much hope :)

[srosset81]

Thanks, I understand it's not easy. I hope that in some not so far-away future, there will be enough SemApps contributors to do everything in the correct way ! It's valuable to have people like you or @Laurin-W which are attentive to this kind of issues.