4 ms·
Well, the method still isn't perfect, due to the lack of easy, well formated, ability to communicate to the contributor why the PR's commits were changed. The c
by timthelion 10y ago
Well, the method still isn't perfect, due to the lack of easy, well formated, ability to communicate to the contributor why the PR's commits were changed. The current UI creates a default culture of really tense back and forth nitpicking comments.
- logingone 10y agoDepending on the size of the nits you might be able to merge as is, then open a new issue to address the nits.
- timthelion 10y agoAnd all of these work-arounds just prove that the system could use a little bit of work.
- eyelidlessness 10y agoI don't agree. If the PR you're reviewing is expected to be merged into a development branch, you can either: 1. Merge without comment. Unless the contributor is closely following development, they won't even notice what you do with the contribution between your development and release branches. 2. Merge with a comment that you'll be cleaning it up before release, and if you want the person to continue contributing explain what you'll be doing. Differentiating stewardship from producing solutions can go a long way toward eliminating debate over minutiae. If there's still tense back and forth, it's probably over approach, understanding of the problem, or awareness of consequences. In none of those cases should the PR be merged. If you want to engage the contribution, explain what the contributor missed and close the PR. If you don't think the contribution is going to be productive, just close it.