I don't think the "single commit" notion is even a good one for an
original PR. I've discussed this with other people and we think it's
okay for the PR to have multiple commits if they serve different
purposes such as renaming variables or restructuring code before
attacking a problem. Likewise it's okay to address review comments in
an additional commit. The PR template just addresses the initial PR
being a single commit anyway.
That being said, if the changes required by reviewers are so substantial
that the PR becomes difficult to review it should be pulled and a new PR
should be created. Otherwise who's going to bother with it?
Regarding conflicts, that's between the contributor and the committer.
A PR that picks up conflicts due to other peoples' work isn't invalid.
Most conflicts are easy to resolve but if it's too gnarly would the
contributor want someone else to do the work and maybe mess up?
On 5/22/18 2:41 AM, Ju@N wrote:
Hello geode devs,
The Geode Wiki has a lot of useful information, not only about the
usage/internal architecture of Geode, but also explanations and guidelines
about how Code Contributions [1] should be made. However, some common cases
are not explained in detail and, as such, contributors (and also reviewers)
might have a hard time trying to deal with these situations in an
*standardize* manner. In particular, I'm referring to the following two
scenarios, which can happen for every single pull request:
- *Reviewers request a change: *do contributors have to make the changes
and just add another commit to the original pull request?. Do
contributors have to make changes and squash everything in one single
commit (executing a *push --force *afterwards)?. Do contributors have to
close the pull request and open a new one with everything squashed?.
- *Pull Request looks good but, after a while without being merged,
somebody else makes a change in develop, causing the original pull request
to become conflictive with the current develop branch: *should the
committer merging the pull request solve these conflicts as well?. Do
contributors have to solve the conflict locally?, if so, do they have to
add a new commit to the pull request or squash everything in one single
commit (executing a *push --force *afterwards)?.
It would be good if we could add these details to the Wiki so everyone
knows how to proceed whenever they hit this situations (git commands will
also be helpful). Would anyone be able to add this to the Wiki?, or reply
to this thread so the approved/recommended process is documented somewhere?.
Cheers.
[1]: https://cwiki.apache.org/confluence/display/GEODE/Code+contributions
