News for hackers. Stuff that matters.
Re:Ask HN: How can I make your life easier when reviewing a PR? by greenyoda on Sunday October 23, 2016 @03:11PM (#12774923)
- Each submission should be a small change that addresses a single issue. It should be easy for the reviewer to understand what you're trying to do.

- Follow the coding conventions established for the project.

- Code quality: factor your code well, use descriptive names, use language-appropriate idioms, etc. The code needs to be easy to understand.

- Make sure you have comments to explain non-obvious things in your code.

- If the project is cross-platform, make sure your code is not dependent on a specific operating system or compiler. If it's web code, make sure it works on browsers other than the one you usually use.

- If it's a new feature (e.g., a new API method), provide documentation for it in the format used by the project.

- Don't gratuitously introduce new dependencies into the project. If the libraries being used have the functionality you need, don't add another library just because you like it better. And don't introduce an entire library just to avoid writing a few lines of code.

- If the reviewer gives you constructive criticism and asks you to re-write your code before they accept it, don't complain about it.

- Perhaps most important: If you have any questions or doubts about what you're doing, ask before submitting the code. It could save both you and the reviewer a lot of wasted effort.

Re:Ask HN: How can I make your life easier when reviewing a PR? by djmashko2 on Sunday October 23, 2016 @02:52PM (#12774823)
I maintain a few open source projects, and found this list to align very closely with my own desires: https://davidwalsh.name/45-github-issues-dos-donts

Some highlights:

- Do submit one pull request to address one issue.

- Do submit two pull request to address two issues.

- Do not tack on a minor whitespace or semicolon changes in unrelated commits or Pull Requests, even if the minor change is correct. Make a dedicated commit or Pull Request instead.

In general, the smaller and easier to understand a PR is, the easier to merge. I've seen a lot of PRs lose their way due to a "while I'm here" mentality of fixing code style etc, and it often obscures the real purpose of the contribution.

Re:Ask HN: How can I make your life easier when reviewing a PR? by clusmore on Sunday October 23, 2016 @07:02PM (#12775851)
>- Do not tack on a minor whitespace or semicolon changes in unrelated commits or Pull Requests, even if the minor change is correct. Make a dedicated commit or Pull Request instead.

Second this point in particular. I work in a small team (4 people) and we used to have a policy of doing code cleanup as we go, but we didn't do very thorough code reviews. After we improved our review policy, now using Pull Requests in Visual Studio Team Services, reviews involving code cleanup became a nightmare - hundreds of syntactic-but-not-semantic changes to dozens of files with a handful of the real changes hidden in there somewhere. We've since changed our cleanup policy so that they must be submitted separately, and even then we try to keep re-architecture and other semantic changes separate from more trivial cleanup, like typos in comments or identifiers, rearranging of methods, etc.

Re:Ask HN: How can I make your life easier when reviewing a PR? by tedyoung on Sunday October 23, 2016 @04:44PM (#12775341)
In addition to the other good recommendations: be responsive. When you get requests to make changes or questions about what you've done, don't just disappear. PRs have a short shelf-life, and if you're not going to respond in a timely manner (even if it's "hmm, I need to think about this"), then hold off on submitting your PR until you have time to follow through on the review feedback.
Re:Ask HN: How can I make your life easier when reviewing a PR? by user5994461 on Sunday October 23, 2016 @06:24PM (#12775717)
What is a PR???

Not sure whether I'm not part of the main audience or the acronym is not as common as you think.

Re:Ask HN: How can I make your life easier when reviewing a PR? by grzm on Sunday October 23, 2016 @06:25PM (#12775723)
Pull request.