Process: Merging PRs

[PapoutsoglouE]

At today's WG meeting, one of the topics discussed was the approach to merging PRs. Right now, only one approval is required before merging.
In some cases, such as editorial changes, this may be fine. However, many PRs would benefit from a broader review. Due to time constraints, this sometimes leads to PRs being merged before more participants have had a chance to review them.

Given the discussion we had, I would take some time to think about this, but here are possible approaches, in my personal order of preference:

1. Decide to not merge non-editorial issues for at least 3 (working) days to give people the opportunity to review
   • or explicitly state that they need more time
2. Use PR templates to keep track of status (incl. checklist)
3. Require that PRs are not merged until they have been discussed at the next WG meeting.
4. Use tools like CODEOWNERS (see Workflow request - Please let PRs slated for discussion remain in "Draft" status #313) - imho this would help a bit, but not with the time aspect

I would like to not spent much more meeting time on this, and ideally to have a decision ready to announce at the next meeting. Until then, feedback and suggestions are welcome!

[HolgerKnublauch]

So far I have followed rule 3 for any non-editorial change. This is for example why I brought up the change to introduce sh:values yesterday, so that everyone is aware of it. I do think it make sense to then wait three more days so that PRs that have at least one approval can be merged. Thursday is merge day. People formally still have until the meeting after to complain if the merge shall be reverted. Nothing is final.

Having said this, I am not a fan of over-engineered processes, and quite often branches depend on each other. For example, the choice to introduce sh:values changes the syntax of lots of examples and (potential) test cases. So now I need to stack branches on top of each other when I want to start working on sh:eval because its test case needs sh:values.

I don't think other open source projects would take two weeks for each change to get into the system. This WG produces a proposal and later in the process as things get more stable, external people will give input too, including (like last time) formal objections. Nothing is final, so I don't think we need to be overly cautious about every intermediate step either.

[afs]

quite often branches depend on each other

This is true at the moment for SHACL Core - presumably (and hopefully!) this will settle down to PR for specific sections.
And for FPWD, we certainly did put in less stable material.

I don't think other open source projects would take two weeks for each change to get into the system.

Two weeks as a matter of course is too long. The alternating telecons does not help matters.

A notice could go to the mailing list.

Some PRs will need a long time, some will not. But 3 days is too short without seeing explicit approvals - most people are not full-time on SHACL 1.2.

Being clear what is coming e.g sh:eval - what else is there? Knowing there is a batch of PRs coming then having a whole document review. It would help to have a "changes" section to help people at the end-of-batch review.

[TallTed]

I don't think other open source projects would take two weeks for each change to get into the system.

There are significant differences between software projects and standardization projects, and we should not treat the latter like the former. Their rhythms are significantly different, by necessity and their very nature.

Open Source Software typically can be patched very quickly when/if an error is found.

Patching Open Standards often takes much more time — months or years, in many cases — after "final" publication. Even during "development" or "maintenance", it can take days, weeks, or even months to get a correction into the document, especially if the members of the group working on the standard disagree about a problem and its best resolution.

Implementation of such standards then takes more time — if they're even actively maintained, at that point — not to mention the part where there's no requirement nor even expectation that implementers monitor published standards for revisions/updates.

(I did support 3 working days in yesterday's call, but I should note that today I have been working through 100+ GitHub notifications, most of which posted between when I knocked off last night and when I picked up this morning, but some of which date further back, because those projects are less "hot" and so can wait longer for my focus. 100+ is a typical overnight. 300+ is typical following a two-day weekend. Longer weekends increase the count.)

[HolgerKnublauch]

I think in the early phases of a W3C process, the edits are much closer to open source projects than, of course, in the later stages. Especially during the experimental development where things get shifted around, I think the process should allow and encourage faster turn-around times. And yes, I hope to be done with SHACL Core rather sooner than later so that the WG can focus on the newer features.

Hopefully, with the sh:values topic out of the way soon, the syntax may be more stable and individual PRs no longer need to be stacked as much as recently.

[PapoutsoglouE]

Good feedback!

Some PRs will need a long time, some will not. But 3 days is too short without seeing explicit approvals - most people are not full-time on SHACL 1.2.

The idea isn't to say that PRs must be merged after three days, but rather that they shouldn't be merged any sooner - and always provided that they have been approved. (Unless you meant something other than Github approvals and I misunderstood.)

It would help to have a "changes" section to help people at the end-of-batch review.

That would be really helpful.

I have worked in teams where the person who submitted the PR was encouraged to be the one to merge it after it has been approved: they are often the ones with the best overview of the situation and what still needs to be done.
There can be exceptions to this (e.g. after an explicit agreement), but how would you feel about such a recommendation?

Having said this, I am not a fan of over-engineered processes

I’d just like to be on the same page about one or at most two general rules recommendations, if we agree that it would help the process.

[PapoutsoglouE]

Let’s monitor whether this becomes a recurring issue and revisit if needed.
In the meantime, the general recommendation is to:

• Be mindful and allow people time to respond to PRs.
• Use your best judgment to notify the mailing list or bring it up in meetings when appropriate.