Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

#ItemDescriptionApproved ProcessNotes

PR WS ScopePR affecting multiple Work Stream (WS) 
  •  Not Allowed
  •  Allowed
  •  Allowed in exceptional cases 

If allowed under exceptional cases, who decides, who approves (all WSLs affected by PR?)

Gergely Csatari : We discussed that the pr should be approved by the sub-project lead of all affected sub-projects.

Pankaj Goyal: the question is are there exceptions for PRs authored by WSLs


PR CH ScopePR affecting multiple chapters
  •  Not Allowed
  •  Allowed
  •  Allowed in exceptional cases 

Gergely Csatari : I think this should not be a problem to allow.

Pankaj Goyal: it defeats the principle of trying to keep a PR "atomic" of only related changes

Karine Sevilla:  preference for 1 PR per Chapter, it's more easy to review


PR Topic ScopePR affecting multiple disjointed topics
  •  Not Allowed
  •  Allowed
  •  Allowed in exceptional cases 

Gergely Csatari : I think this should not be a problem to allow.

Pankaj Goyal: it defeats the principle of trying to keep a PR "atomic" of only related changes

Karine Sevilla: preference for 1 PR per topic, it's more easy to review


PR Comment ResolutionWho is authorised to "resolve" a comment
  •  Comment Author
  •  PR Author
  •  WSL if Comment Author unresponsive and WSL not PR Author
  •  Another Active Contributor if WSL is the PR Author

What is the period of unresponsiveness?

Gergely Csatari : According to CONTRIBUTING.md:

"Comments/sub-Conversations within a PR may only be marked Resolved by:

  • A) The originator of the sub-conversation.
  • B) The relevant WSL or
  • C) An automated timeout of 48hrs when there has been no additional comments after an update is posted to resolve the sub-thread.
  • The person responding to a sub-conversation shall not mark it as Resolved."

Pankaj Goyalshould we change the period of "48 hours" to, say, at least a week?

Pankaj Goyal Should we clarify "relevant WSL only if they are not the respondent of the sub-conversation"?  

Karine SevillaI agree to change the delay to one week before closing


PR Change Resolution
  •  Change Author
  •  Who if Change Author unresponsive 

Gergely Csatari : Should not matter who is making corrections until the corrections are done.

Pankaj Goyalthe corrections may have been made bu the "change requestor" has not resolved – sometimes for weeks even with repeated reminders


Minimum duration a PR needs to be open for maximising input after changes complete
  •  1 week
  •  2 weeks
  •   

Or should it be Minor 1 week, all others at least 2 weeks?

Gergely Csatari: There is a 2 days cool off period mandated by the current CONTRIBUTING.md I do not see a reason to change it.

Pankaj Goyal: 2 days worked when Rabi was 24 hours a day tracking and responding. But even then he would make a request and folks were much more responsive. We should have some mechanism that asks for at least a reminder and then resolution but 48 hours is not enough. Secondly, any period should be in terms of business days, taking into account that a request from a certain region in the US to, say, Australia could already be 19 or so hours."


PR Re-Approvals Ignore approvals made prior to non-typo changes
  •  Required After non-typo changes
  •  Not required 

Gergely Csatari: Arent't these automatically cleared after a new commit is pushed?

Pankaj GoyalIt isn't automatic - there is a recycle symbol but it has to be manually invoked.


PR Merge – non WSL author
  •  WSL

Karine SevillaWSL + 1 major and active contributor of the WS


PR Merge – WSL author
  •  WSL - no additional steps
  •  Final Additional Reviewer to ensure all comments/changes addressed and processes observed

Final Additional Reviewer to comment that they are OK with the PR being merged. They would have verified that the above agreed-to processes have all been observed.

Karine Sevilla WSL + 1 major and active contributor of the WS