docs: Update releasing notes to reflect that main branch is now protected

[binste]

I think protecting our main branch is a great idea:

• to prevent accidental pushes to main
• while tests are still running for a PR, you can now use the auto-merge feature. I find this very convenient in other repos as you don't have to check back in a few minutes and then merge but instead, if you click it, it merges automatically once all checks have passed:
  

I took the liberty to already enable it in the repo settings based on #3559 (comment). This PR updates the RELEASING.md file to reflect it.

We can still merge without review which I think is useful for minor changes (typos, minor changes in pyproject.toml, etc.)

Happy to discuss! We can also try it out for a while and always revert back. Tagging for visibility in case you want to weigh in: @dangotbanned @joelostblom @mattijn @jonmmease

[dangotbanned]

@binste I'm generally +1 on this, but had a couple of questions.

1. Would you be able to show which settings were selected please?
   As I'm not an admin, I can't see our config for this.
   Looking at about-protected-branches there seems to be quite granular controls, not sure how relevant all of them are though.
   Non-admin repo settings

   

We can still merge without review which I think is useful for minor changes (typos, minor changes in pyproject.toml, etc.)

2. Would the protected branches feature support rules (or alternatively, adding a short set of guidelines) on merging without review?
   I'd feel more confident when deciding to merge without review, if I could refer to something first (that everyone has agreed upon).
   Mainly looking to avoid any unwanted surprises in the future.

Example

I thought these would require a review:

• test: Adds test-(slow|fast) options #3555
• refactor: Reduce SchemaValidationError traceback length #3530
• feat: Adds @register_theme decorator #3526

But these I thought would not:

• ci: Add typings/ to .gitignore #3560
• refactor: Replace an indirect SchemaBase import #3556
• test: Shorten test_chart_validation_errors test ids #3551
• test: Rename test to more specific chart_error_example__four_errors_hide_fourth #3546

Looking specifically at #3555, merging is now blocked due to conversations I (the author) raised.
If I were to resolve these and then merge without review, someone looking back at the PR might have questions that are easily answered by the (now collapsed) "resolved" conversations.

I'm not sure what the solution is, but I think this requirement could have the side-effect of lowering the level of this kind of documentation.
Since adding comments would now require more scrutiny than a PR with none

[joelostblom]

LGTM! I'm not familiar with rulesets so can't comment much on that

[binste]

Thanks for the inputs! Here are all branch protection rules for main:

I've never seen rulesets apart from code owner. So far, I think it worked well without making it explicit what we think should require a review, requesting reviews if in doubt to be on the safer side. If we don't feel comfortable with a merged PR, we can always go back on it and discuss it before making a release. Also, if someone goes a bit over board with merging unreviewed PRs, we can also bring it up :)

I do see the value of making this explicit with more maintainers but I find it difficult to come up with good rules. Generic ones might be too restrictive such as if something touches pyproject.toml or altair, it requires a review, if it's about tests then you can skip it. This would stipulate that #3556 would have required a review but I agree with you that this looked like a safe change without the need for someone else to go over it.

Regarding "resolved" conversations, when looking at past PRs to understand some changes/parts of the code, I often look into resolved comments to see what the discussions were around at the time. I don't mind expanding them. When working on a live PR, I appreciate the opportunity to mark discussions as "resolved" as else I find it too easy to miss an open point.

I'll merge this but happy to discuss proposals for rule sets or changes to the settings in a new issue! :) The repo should work for everyone.

[dangotbanned]

Really appreciate the writeup thanks @binste - all sounds good to me