What's the best (or correct) way to perform a code review and why?
Open SVC (Github, Bitbucket, Gitlab.. or any code review tool) and look into code and make some comments
Pull the source into your machine, check the code, made some basic test, and make some comments on code review tool
We would say both. Firstly, I read the code and leave comments on GitHub. Then I pull the changes and try to run the app locally.
By the way, most replies (including mine) were centered around the 'where' (online, or pull). But since the title is 'how', I think it's a good opportunity to share this good talk from Nina Zakharenko. Most of it is really generic and language agnostic, and 90% of the talk will still be relevant to people not doing any Python at all.
How to be a good PR reviewer. And how to be a good PR submitter. (Spoiler; both could be hard, because of people, but nothing a walk or a coffee break couldn't resolve)
At work, I use a code review tool, without pulling in the code except in special cases. We have testers who will verify that it works; the review is mostly for conventions, code quality, architecture, seeing if things make sense... If it needs running the code, it's the job of the developer and the tester.
For open source projects, I don't have testers, so I more often pull the code to test locally. This is besides using the code review interface of Github / Bitbucket.
Perhaps if I'm ever in charge of this choice, I'd look for a code review tool with good IDE integration. I'd like to test code and leave comments without leaving the IDE.
I prefer to pull as I dislike the idea of working on live codebases -- BUT local testing often doesn't line up with what a live copy spits out when running due to server differences, so I would actually say both.
Though as a rule of thumb I don't like version control software and avoid it, I prefer dealing with environments where you have a project manager that does their flipping job! the past decade and a half as VCS has been thrown at EVERYTHING I've watched far too many codebases get tanked by it thanks to the lack of direct communication between the people working on things, lack of oversight, and lack of any real chain of command.
I'm not saying it shouldn't be used or doesn't have a place, but unless you have a benevolent dictator sitting atop things riding herd, far FAR too often it can be as bad if not worse than going without.
Like Jake wrote, I'd vote for both. Depending on the codebase, the RP size and the author, I could only review it online, or pull it to review locally, or both...
You need an option for both! I look at the code visually in github before pulling it down because sometimes I can reject it before spending the time and energy pulling it down to test locally.
Rohith Kunnath Puthan Veedu
Backend Developer
Really likes Upsource, because can easily integrate with Intellj and take actions.