Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ready for review] Updates to A Reviewing section #449

Merged

Conversation

illushka
Copy link
Collaborator

Summary

Added additional content in order to point out some more tips on how to make code reviews even more friendly, efficient as well as how to avoid subjective disagreements.

What should a reviewer concentrate their feedback on?

Just check if the new text fits with the old one. I checked for repeated content but another eye to ensure nothing is repeated may be useful.

Acknowledging contributors

By submitting this pull request I confirm that:

  • [ x] all contributors to this pull request who wish to be included are named in contributors.md.

@rainsworth
Copy link
Collaborator

Thank you so much @illushka ! We will get this reviewed ASAP 🎉

@rainsworth rainsworth changed the title Updates to A Reviewing section [Ready for review] Updates to A Reviewing section May 15, 2019
@rainsworth rainsworth requested a review from yochannah May 17, 2019 12:03
@rainsworth
Copy link
Collaborator

Pinging @yochannah to review these contributions!

@yochannah yochannah self-assigned this May 17, 2019
@netlify
Copy link

netlify bot commented May 17, 2019

Deploy preview for the-turing-way ready!

Built with commit 521a5ae

https://deploy-preview-449--the-turing-way.netlify.com

@@ -85,10 +90,23 @@ Reviews conducted in the right spirit (see especially [here](#Be_nice)) also ser

As with all open-source and collaborative enterprises, good internet etiquette makes the whole process go more smoothly. Perhaps most importantly, always assume good faith on both sides of the review interaction, and always be constructive. These principles are true for the review process beyond almost any other project aspect, since it necessarily involves criticism, potentially between two complete strangers. If you're the coder, don't waste your reviewer's time. If you're the reviewer, listen to what the coder is telling you in reply, and work collaboratively with them to make the code better.

### Avoid being subjective
Code reviews should strive to be as objective as possible. Of course, subjective coding preferences may come up in any project. However, such preferences wherever possible should be decided at the project level beforehand. Thus, one can avoid the situation where an opinion might be passed of as fact. Instead suggestions can be supported by pointing to documented preferences that have been set up in advance. If you do come across undocumented preferences, discuss them with the team again and agree if you would like to add the preference to the checklist of your code review process.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the guidelines about documenting style guides in advance to avoid arbitrary personal preferences!

Code reviews should strive to be as objective as possible. Of course, subjective coding preferences may come up in any project. However, such preferences wherever possible should be decided at the project level beforehand. Thus, one can avoid the situation where an opinion might be passed of as fact. Instead suggestions can be supported by pointing to documented preferences that have been set up in advance. If you do come across undocumented preferences, discuss them with the team again and agree if you would like to add the preference to the checklist of your code review process.

### Specify crucial versus optional changes
You might want to differentiate between changes that are crucial and changes that are nice to have. For example, comments that begin "You might..." could be used to express suggestions the reviewers want the coder to consider but are not essential. These can be particularly useful to guide inexperienced coders to write better code while not being too nitpicky. The coder can then decide to ignore these non-crucial comments if they don't agree. Reviewers could use comments that begin "You must..." to specify those that are not optional.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, love the guidance for providing suggestions without being nitpicky over the less important bits 😻

Copy link
Collaborator

@yochannah yochannah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are some fantastic guidelines to increase good quality, friendly commnication while still enforcing quality - I love it!!

@yochannah
Copy link
Collaborator

@illushka This is a great contribution - @KirstieJane will add you as a collaborator to the repo, then you can go ahead and merge this!! 💯

@illushka
Copy link
Collaborator Author

Okay great!

@rainsworth
Copy link
Collaborator

@all-contributors please add @illushka for doc

@allcontributors
Copy link
Contributor

@rainsworth

I've put up a pull request to add @illushka! 🎉

@rainsworth
Copy link
Collaborator

Hi @illushka, you have been added as a collaborator to the Turing Way repo so you should be able to merge this in yourself now! Thanks so much for contributing 🎉

@illushka illushka merged commit e1527c4 into the-turing-way:master May 27, 2019
@dingaaling dingaaling mentioned this pull request Oct 31, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants