Project ownership / roles

[rsvoboda]

We need to define a bit more clearly project owner(s) and PR captains/reviewers.

There should be also clear distinction between https://github.com/orgs/jboss-eap-qe organization members and https://github.com/orgs/jboss-eap-qe/teams/mp-team members

[fabiobrz]

Hi @rsvoboda, any news on this?
Can we agree on some day 0 setup and then adjust things as it's needed? Goal is to have quicker interaction on PRs.

[rsvoboda]

This is planned for Wed mtg discussion

[fabiobrz]

@rsvoboda we definitely need to improve iteration over PR review. My "intial reviewer" proposal is not speeding up PR processing. This is to be solved asap, IMO.

[rsvoboda]

Do you mean mainly #91 ?

I think your "initial reviewer" proposal is good.
We need to get to the speed and work on morale to check PRs frequently.
If you need PRs to proceed quickly, you and the reviewer for the day should be in touch even prior to PR submission.

[fabiobrz]

Do you mean mainly #91?

Mainly, there are also examples of even slower PRs.

We need to get to the speed and work on morale to check PRs frequently.

What is needed in order to do that? Initial review is just about that. !immediate", quick and to be handed over to the official reviewer/merger.

If you need PRs to proceed quickly, you and the reviewer for the day should be in touch even prior to PR submission.

Disagree. It would definitely be another mean of slowing down things. Initial review is just about that: having one eye on the list, stopping by as the first one comes in and review.

[rsvoboda]

You and the reviewer for the day should be in touch even prior to PR submission - I mean just to be aware such payload is coming and he/she can plan a bit for it.

We don't have hard requirements for review feedback response and I don't want to go that way as we don't have capacity guarantees/allocation for it.

[rsvoboda]

OK, trying an optimistic way now, merging PRs as they got at least one guy reviewing them even though they don't have official GH approval.

[fabiobrz]

You and the reviewer for the day should be in touch even prior to PR submission - I mean just to be aware such payload is coming and he/she can plan a bit for it.

We do are in touch. We have mail, chat and notifications from Git Hub, definitely no more time should be devoted to communication. "Initial reviewer" concept is just about monitoring the queue and act quickly.

We don't have hard requirements for review feedback response and I don't want to go that way as we don't have capacity guarantees/allocation for it.

Indeed. But we have hard requirements for merging as there's just two people able to write to the repo.
This is fine to me but then - IMO - then review/merging should happen quickly.

[fabiobrz]

OK, trying an optimistic way now, merging PRs as they got at least one guy reviewing them even though they don't have official GH approval.

This decision - IMO - shifts the balance from one edge to the opposite one. Proposal: initial reviewer + GH approval? Rationale: one tem member could make a multiple round of initial review (I did for #87, I think) so let him/her finish their work consistently and place that GH approval.

I agree that we can drop the part where the review is handed over from the initial reviewer to the the merger for another round, this is not needed. Let the merger have a quick recap of the review and that's it.

[mnovak]

Based on discussion above and team chat, suggested approach would be following:

1. There will be initial reviewer (reviewer of the day) who reviews PRs
2. Once reviewer of the day granted approve then PR is passed to PR merger (@mnovak1) to approve and merge.
3. If PR merger is OOO/unavailable then there is second PR merger (@rsvoboda).
4. If backup for PR merger is OOO/busy then pass PR to someone else from @jboss-eap-qe/mp-team to approve and merge

[fabiobrz]

Hi @mnovak1, thanks for summing up my proposal and let me add some details about the process, as agreed in our chat discussion:

1. PR reviewer of the day picks up the review: quickly.
   Rationale: The fact that it is on daily basis means it comes before his/her own development. As he/her sees a PR, he/her should stage his/her work (consistently - let's say in half an hour or so) and then process the PR, reviewing it and doing more rounds with the author.
2. Once this is done, then he/her should place the GH approval for the PR to be handed over to the merger.
3. He/her, in turn, should just have a quick recap - i.e. they could read the discussion or comments and so on, to check, without need for another review (there can be exceptions) - and then merge.

That said, @rsvoboda and @mnovak1, so you suppose you'll be able to grant write/merge rights "on-demand" when you'd be unavailable or just grant those rights once and "forever"?

[mnovak]

@fabiobrz thanks for the notes. To point 3. above...I hope it will be the case that merger will do just quick recap but I guess it will depend how good to the initial review was and will need some education of team members.

you'll be able to grant write/merge rights "on-demand" when you'd be unavailable or just grant those rights once and "forever"

I'm more for 2nd option to grant rights forever to jboss-eap-qe/mp-team as we will not need to check who's here or not. Only in case that this will be misused (above process not followed) then we'll reconsider it.

[fabiobrz]

Hi @mnovak1

@fabiobrz thanks for the notes. To point 3. above...I hope it will be the case that merger will do just quick recap but I guess it will depend how good to the initial review was and will need some education of team members.

Right, I agree.

I'm more for 2nd option to grant rights forever to jboss-eap-qe/mp-team as we will not need to check who's here or not. Only in case that this will be misused (above process not followed) then we'll reconsider it.

Perfect, thanks for making this clear.

[rsvoboda]

Hi @fabiobrz, regarding "This decision - IMO - shifts the balance from one edge to the opposite one."
I added a message to the PRs (and here) about the motivation for that one time action, you already escalated twice that you are waiting for PR merge. Many people are on PTO or have PTO in a few days, everybody wants to finish their work.
You don't like waiting for the time-consuming review process, you don't like my action from yesterday to unblock current pain (waiting PRs). We have our group meeting tmr, please prepare a few steps how would you solve the situation from yesterday in my place.

[fabiobrz]

Hi @rsvoboda, will do, thanks.

[rsvoboda]

Closing, we are tracking this in working group document.