search

chexxor / purescript-documentation-discussion

A place to organize the planning of improvements to the PureScript documentation.
7 stars 0 forks source link

Workflow question: PRs and merging them #36

Closed JordanMartinez closed 5 years ago

JordanMartinez commented 5 years ago

I believe we have already agreed to only submit PRs for potential issues that need further discussion.

However, once we reach agreement, who should merge the PR and when? Do we want to indicate anything like "let me merge it when I'm ready?" or things like that?

As an example, I'm ready to merge #26, but I don't know whether this is something you expect me to do or not. If you were waiting for me to review and comment on it and I have done so without any problems, should I just merge it?

chexxor commented 5 years ago

Good question, because I've seen two ways of managing PRs. In a team project where everyone has commit rights and PRs are seen primarily as a code review system, then a PR is merged by the person who opened the PR after getting enough approvals. In a community project which has fewer people with commit rights than contributors, PRs are merged by people with commit rights at their leisure. I believe in that latter situation the maintainers will wait for the PR creator to give a final thumbs-up before merging, if it's not clear or obvious that the PR opener is completely finished with their patch (it's what I would do).

For our case, because the PR is serving as part of a review system, then it follows the pattern of that first case I described. So, I think the PR opener will merge it themselves. Does it sound OK and make sense to you?

JordanMartinez commented 5 years ago

Yeah, let's do the first one since it is our way of a review system.