Agree on review / merge guidelines

[Potherca]

As MRs are being opened and work commenses, I thought it might be good idea to address the topic of whom is "allowed" to merge. (As this is actionable, I created an Issue, rather than a Discussion)

I've seen this become a bottleneck in other projects caused by bystander apathy.

As we want to help @RicardoNiepel by taking over some of the load, my proposed guidelines are:

1. Add the most active developers for a review (Currently @adrianvlupu, @aheil, @IOrlandoni, and @RicardoNiepel)
2. When an MR has X approvals from those developers it can be merged.
3. The author of the MR should merge their own MR (in case of conflicts they will be best suited to resolve things), unless someone else is waiting for the work from that MR.
4. Don't worry about things too much. A merge to master is not the same as a release. There is always time to correct (or undo) anything that goes in the wrong direction. Better to keep moving forward than to have to wait for everyones approval.

I know all of this is implicitly more or less the way of doing things but I think it would be good to make it Explicit.

So what do you say? Agree/Disagree? Suggestions for improvement?

Tasks

• [x] Agree on how many "X approvals" should be

• Get everyone's feedback and approval:

  • [x] @adrianvlupu
  • [x] @aheil
  • [x] @Crashedmind
  • [x] @IOrlandoni
  • [x] @RicardoNiepel
  • [x] @stawirej

[adrianvlupu]

Had no idea you can use checkboxes 😊

I agree with all the above points.

I don't think @RicardoNiepel will have time to give approval but everyone else can join the process. I would be inclined to get the opinions of @aheil and @stawirej before a merge, seeing as they use it to communicate to lots of people.

[RicardoNiepel]

Perfectly fine for me - would recommend min 2 reviewers from the core team.

And make automatic tests required when they are merged/available.

[aheil]

Sounds absolute reasonable to me. Also I second "don't worry to much". 😃

[Potherca]

@Crashedmind Can we get a Yay/Nay from you on this?

As we seem to be in agreement on this, I would suggest creating a CONTRIBUTING.md and writing them down there (either in this repo or in a organisation-wide .github repo 🤔)

[kirchsth]

If you want, you could test your review rules, ... on my extensions. ;-) I'm basically finished with my extensions and would start with an update of the https://github.com/structurizr/dotnet-extensions/tree/master/Structurizr.PlantUML/IO/C4PlantUML as soon the macros(),... are fix.

[kirchsth]

Can we extend the review guidelines with a cyclic check of all pending PR's (e.g. 2 weeks). If you want, you could test the extended review rules, ... on my PR's ;-)

[Potherca]

Regarding 2 reviewers from the core team.... This is becoming a bottleneck for changes being merged.

Some PR are open so long that the stale-bot is triggered!

I think we either need to find a way for contributors to actually commit to reviewing, add a time-limit for how long an MR has to wait for a second reviewer, or reduce the limit to 1.

[kirchsth]

@Potherca: Can we create a new release (independent of other reviewers) that the new features can be merged into PlantUML and it can be used with kroki too (#156)?