In general, you must review everything. If a fresh application has 2 000 LOC, all 2 000 LOC must be reviewed.
That's why there is no best practice on how to choose what to review.
If you approach an existent large codebase, never reviewed before, then it's the same thing when you must rewrite an existent large codebase, and to choose where to start. It strongly depends:
on the codebase (a single monolithic code would be more difficult to rewrite/review than a set of separate components, etc.),
your context (can you stop everything you work on and spend three months (three years?) working only on rewrite/review, or you must do it by small lapses, only when you have free time)?
the type of review you do (do you have a checklist of things to review? Depending on the items of the checklist, you may want to review some parts first).
If I were you, I would:
follow the 80%-20% principle, mentioned in the first comment of the second question you linked to.
take in account that 100%, being an ideal, isn't maybe worth it. It's like 100% code coverage for unit tests, except that such code coverage is mostly impossible or extremely expensive.
start with the parts of the code you use the most and which are the most important. If the codebase has a library which authenticates and registers new users on your corporate website, review it first, because you want certainly find security holes before hackers do.
use existent metrics to determine what is more important to review. If a part of the codebase has no unit tests at all, while another, equally important part, has 85% code coverage, start by reviewing the first part. If a part of the codebase was written by a developer who was known to be inexperienced and to introduce more bugs than any of his colleagues, start by reviewing his code first.
From what I've seen, most of those steps are done on Github by convention, and not by any official Github-provided process.
My employer uses Github, I run a good number of small open source projects, and make occasional contributions to other open source projects.
Here is how I've usually seen it done:
Author adding his/her colleagues as reviewers:
This varies from project to project, but in general, the assigned peer reviewers are all of the contributors to the project.
Open source projects seem to have a rough hierarchy - maybe their convention would be to only merge after a "core" contributor has given the okay.
At the shop where I am currently employed, we merge after any one of the half-dozen developers on the team has given their approval.
On rare occasions someone on the team may use a comment to specifically call out another developer that they think should peer review the code before it is merged, but otherwise, whoever gets there first and feels like doing so can review and make comments.
Reviewer approval:
Approval is typically shown by making a comment on the pull request saying "+1" or "lgtm" (looks good to me).
Lightweight tasks:
I've used the checkboxes as well, but in most cases, every comment on a pull request is considered an implicit "task" that is resolved either by:
- changing the code that the line is commenting on
- responding with another comment
Seeing at a glance what is approved and what still needs reviewed:
I have used the Looks Good To Me extension for Chrome, which gives you such a view from the Pull Requests screen. The pull requests list view seems to have been broken by recent Github changes, though.
Best Answer
There are several relevant trade-offs here:
In general, if the refactoring is really outside the scope of the branch I would recommend making it a separate branch.