Clarify how to review on GitHub

[louisom]

the pull request review step is here: http://cpython-devguide.readthedocs.io/pullrequest.html#reviewing

But it didn't involve how to review on GitHub, maybe it should be update for how to review on GitHub (the step, and how to request change ...etc)

[brettcannon]

If you're suggesting explaining how to use GitHub to leave a review, we are purposefully not doing that as we don't want to duplicate GitHub's own documentation on how to use the site.

If you're suggesting something else, @lulouie , I unfortunately don't know what it is.

[terryjreedy]

The devguide could link, for instance, to https://help.github.com/articles/reviewing-changes-in-pull-requests/. The Help link at the bottom of Review pages is to the general index. It took me a while to find this link.

[brettcannon]

Yep, linking is fine and a good idea.

[louisom]

@brettcannon yes, a link will help others, since devguide didn't mention any step on GitHub, only local's.

[berkerpeksag]

IMO we shouldn't really add links to every single article on help.github.com. They can easily be found by using Google or the search feature of help.github.com. There is no need to clutter the devguide with every single and trivial details of Git and GitHub.

[willingc]

I believe that this issue's goal is to assist core devs (or future core devs) how to use GitHub and git when doing a review. I imagine that whatever links are proposed in the PR for this issue will be ones that are helpful to achieve that goal.

[berkerpeksag]

@willingc, I'm pretty sure that almost all of the core devs used a review tool before and they are capable of doing their own research when they need additional information. Please stop treating us like idiots.

Devguide is already full of duplicate content. Accepting every single issue/PR is not going to make it a better resource.

[Mariatta]

I'm with @berkerpeksag on this. There is already a link to GitHub help page at every Github pull requests and issues.
I suggest, if we want to add any link at all, to refer people to GitHub help page, and if they need further help on how to use GitHub, go to that page first.
Maybe the link to GitHub help page can be added here: http://cpython-devguide.readthedocs.io/#contributing

[willingc]

Please stop treating us like idiots.

@berkerpeksag That comment is a bit harsh and out of bounds.

The devguide's purpose is to serve the developer community. Terry had requested information and @lulouie was trying to accommodate the request on the core mentorship list. Of course, there's a balance needed between too much and not enough information. I agree with @Mariatta that the link to the help page would be useful.

[Mariatta]

Thanks @willingc and I'm really sorry for this.
To clarify, I agree with @berkerpeksag that we shouldn't be adding every single link to GitHub help page, not to the rude behavior and the use of harsh language.

Please remember that PSF Code of Conduct applies to core developers too.

I am thankful and welcome everyone's contribution, but we can't always accept every single idea or pull request that comes through. It takes time and effort to consider whether an idea/PR should be accepted or rejected. When we decide to reject, we should convey the reason in positive manner.

There are various ways to contribute to our community, and it doesn't always involve writing codes and patches. Sometimes being kind and supportive to each other is all we need.

[terryjreedy]

The link I suggested was carefully selected as the link to add to the reviewing section of the devguide. It is an overview page that links to 7 specific pages. It is enough for this issue.

It also has a link back up the TOC tree to https://help.github.com/categories/collaborating-with-issues-and-pull-requests/, which is a pure sub-TOC page. This might be added the section on 'Life cycle of a PR', whether as part of this issue or another.

The problem with the Help link at the bottom of PRs is that is goes to the 20 +/- screen (depending on font size) master TOC that starts with multiple topics irrelevant to any of us. The 'Collaborating' section is just below the middle and the Reviewing part at the end of that.

[berkerpeksag]

Well, I didn't know that CoC can be used as a weapon to intimidate someone who disagrees with you.

I believe that this issue's goal is to assist core devs (or future core devs) how to use GitHub and git when doing a review.

@willingc feel free to share your opinion on what would be useful for future core devs, but please keep core devs (or at least just me) out of it. I can find my own way without an assistance or I can ask for help when I needed.

[willingc]

Thank you for the feedback. I can see that my thoughts and effort would be better spent on projects that need contributors.

[brettcannon]

OK, this issue has gotten out of hand. I have locked the issue to prevent others from contributing who have not already done so.

@berkerpeksag I think exception was taken with the phrase "Please stop treating us like idiots" because in North America that's taken as an insult. As for @Mariatta mentioning the CoC, it was to act as a reminder that us core devs have to be attentive to how we phrase things just like everyone else because the phrase you used came off as insulting, not as a threat to intimidate you since Mariatta actually agreed with your position.

@willingc I really hope you don't leave, but if you go I'm sorry to see it happen and thanks for all the work you have done.

[willingc]

Thanks @brettcannon.

I'm sorry too. I can write this particular issue off as a common misunderstanding of words or intent. If anyone felt any ill will by me, I am sorry about that.

My intent was simply to point out that it's good to have empathy for all users of the devguide whether or not the user is a core dev or new contributor. While I can see how core devs might think that specific directions/links may be clutter, I can also see the viewpoint of others appreciating a link that helps guide them somewhere relevant. Since there is more than one target audience for the devguide, perhaps it's time to refactor it to better suit these audiences.

[brettcannon]

@willingc I have opened #189 to clean up the index page in hopes that will clearly separate the audiences. That should also help show us any redundancies we have to help prune down the information to make the overall guide more manageable.