TE
科技回声
首页24小时热榜最新最佳问答展示工作
GitHubTwitter
首页

科技回声

基于 Next.js 构建的科技新闻平台,提供全球科技新闻和讨论内容。

GitHubTwitter

首页

首页最新最佳问答展示工作

资源链接

HackerNews API原版 HackerNewsNext.js

© 2025 科技回声. 版权所有。

Code Reviews: A Layered Workflow

31 点作者 pizza_pleb将近 6 年前

3 条评论

Smotko将近 6 年前
The article makes some good points. Here are my thoughts:<p>&gt; 1. Preliminary Checks<p>+1 for this, but I would also add that you need to add as much formatting&#x2F;linting into this step as possible. You shouldn&#x27;t be pointing out formatting issues in your code review.<p>&gt; 2. Understanding<p>This is super important! Pull requests that do not explain their purpose in the description should not be reviewed.<p>The PR description is also really important when git bisect leads you to the PR when debugging a new bug.<p>&gt; 3. Usability Test<p>A lot of debate regarding this, but I believe this is extremely important if not the most important part of your code review. For details read this section[0]<p>&gt; 4. Code Review<p>A few good tips in the article, I&#x27;d just add that you should be polite and try not to waste people&#x27;s time.<p>[0] <a href="https:&#x2F;&#x2F;blog.codereview.chat&#x2F;2019&#x2F;06&#x2F;27&#x2F;code-reviews-and-your-company-goal.html#1-catch-code-defects-before-the-code-is-shipped-to-qa-or-prod" rel="nofollow">https:&#x2F;&#x2F;blog.codereview.chat&#x2F;2019&#x2F;06&#x2F;27&#x2F;code-reviews-and-you...</a>
评论 #20535918 未加载
rinchik将近 6 年前
IMHO, #3 - Usability Test should never be part of the code review, the author of the PR should make sure that changes do what they are supposed to do <i>BEFORE</i> opening a PR.<p>Developers should be responsible for their own QA. QA is not what code reviews are about<i>.<p></i> There are always exceptions, of course. Use of the common sense should be highly encouraged, e.g. if proposed change is complex or reviewer want to visually confirm this change, feel free to checkout this branch, but it is <i>NOT</i> a requirement for a good review.
评论 #20535532 未加载
评论 #20535861 未加载
gregmac将近 6 年前
I&#x27;d add to the &quot;Understand&quot; part: should include samples (screenshots, logs, output files, etc), where appropriate.<p>This helps with understanding for the review, and also if it&#x27;s referenced again in the future (eg, someone is coming back to fix a bug that&#x27;s later found to be introduced by the change). It takes only a minute or two extra to do this, while saving the reviewer a lot of time (checking out and running) and&#x2F;or mental gymnastics (reasoning about what CSS&#x2F;html changes will actually look like).<p>Obvious example of where this is useful is for a UI change (with before+after and&#x2F;or annotated screenshots), but I&#x27;ve also found this useful for timing bugs (eg logs), and data-driven code (eg where output depends heavily on data).