Stream: t-cargo/PubGrub

Topic: Merge Policy


view this post on Zulip Eh2406 (Jan 14 2021 at 20:25):

Currently we do not merge work without explicit approval of all project members.

We have been taking turns being unavailable to review each other's work. I am finding it as a stressor in my todo list, knowing that some things are waiting on me. I also understand that it is probably demotivating knowing that your work won't get merge for sum indefinite period of time until I get back. So I think it is time we renegotiate our merge policy.

view this post on Zulip Eh2406 (Jan 14 2021 at 20:28):

I wanted to bring it up now to give us time to come to a shared understanding before "the end of january" period that Matthieu mentioned.

view this post on Zulip Eh2406 (Jan 14 2021 at 20:39):

To start the conversation: If someone has not responded in 10 days ( long enough that one can check in only on weekends ) then that can be considered approval. At that time things can proceed as if the absent party had sent "sounds good, do what you want". Even if no one responds a PR should not be merged by its author. If someone objects to a PR then that needs to be fixed or responded to. Only after it is addressed does the timer restart. If you have thoughts but do not have time to write them up, a comment like "I want to get back to this." will restart the timer, setting you back to having 10 days to respond.

What do you all think?

view this post on Zulip Matthieu Pizenberg (Jan 16 2021 at 11:44):

Even if no one responds a PR should not be merged by its author

I'm puzzled by this sentence. How can a PR be merged if no one responded in 10 day yet its author cannot merge it? Could you detail your thinking please?

I'm ok for a 10 days policy. Having us all review everything was important for the beginning of the project. Now we've reach the difficult "long term project" barrier where people are not as excited as when beginning and don't have as much time. I personally have been focusing the free time I had on elm-test-rs to finish what I started for exercism. That's also why I'm glad we have this in an independent repo! I'm not done with pubgrub-rs, that's for sure! I'll continue to help where I can (starting with things here https://github.com/pubgrub-rs/advanced_dependency_providers/issues/5). But yeah it might take some time before I can refocus some of my energy there.

view this post on Zulip Eh2406 (Jan 16 2021 at 16:35):

How can a PR be merged if no one responded in 10 day yet its author cannot merge it?

I was thinking that if literally no one responds, then it should not be merged. I would still like code to be reviewed by someone that is not the author. But relax the "we all agree" to "all available contributors agree".

view this post on Zulip Alex Tokarev (Jan 17 2021 at 16:55):

Sounds good, that's why only 1 required approval was set up btw.

view this post on Zulip Matthieu Pizenberg (Feb 08 2021 at 21:43):

@Eh2406 I've had a busy weekend sorry, I'll have a look at your PRs next weekend

view this post on Zulip Eh2406 (Feb 08 2021 at 21:43):

No probem.

view this post on Zulip Eh2406 (Jun 06 2021 at 03:22):

Now that we have some experience with this, I am wondering whether we can let things go faster when both Matthieu and I have review a PR? If someone spots a problem we can always revert.

view this post on Zulip Matthieu Pizenberg (Jun 06 2021 at 15:59):

We could indeed shorten the delay when we have both reviewed a PR since we are the two persons knowing well the whole code base. But to be honest I don't mind the delay. In a sense, a slower pace almost feels like less pressure and more long-term sustainable. It might be annoying sometimes though for PRs changing the same piece of code with potential merge conflicts

view this post on Zulip Eh2406 (Jun 06 2021 at 16:30):

Good point on sustainability, let us leave it as is. It can be a reminder to take brakes and have other priorities. :-P


Last updated: Oct 21 2021 at 20:47 UTC