I don’t pull it to run it. I pull it if I need to navigate the PR in an IDE.
Github beta has file tree for PR's, really cool to avoid for such cases https://github.com/github/feedback/discussions/12341
Ask HN: Do you pull and run code as part of code review?
41–50 of 146 posts
Re: Ask HN: Do you pull and run code as part of code review?
#42I work on a frontend team, you usually want at least 1 person on the team who reviews code by pulling and running it. They can find unique bugs you didn’t think of while coding. For me, if I’m not busy I try to run it, if I’m busy I probably only give it a quick read. I try to maintain one development server whose purpose is to host my in review CR’s code so that reviewers can just click it and try it instead of having to pull it.
Re: Ask HN: Do you pull and run code as part of code review?
#43Generally, no, it’s CI’s job to build and run tests on every PR. On some occasions when tests do not cover some condition, I ask to add the test, instead of running it myself.
Re: Ask HN: Do you pull and run code as part of code review?
#44We have a CICD pipeline for every branch. We use squash.io which spins up a VM and runs the docker compose to create a per-branch QA env. The process however is that a developer does a code review on just the code first, then a tester tests the feature branch, then we merge and do some regression testing (manual and auto), then release.
Re: Ask HN: Do you pull and run code as part of code review?
#45Only very rarely. Because we have a dedicated QA team (normally an antipattern but for complex enough software you some times end up building software you literally can’t run which is - yes - weird and difficult).
Re: Ask HN: Do you pull and run code as part of code review?
#46This should be something agreed within a team, so that review standards are consistent across team members. In my previous team, the reviewer was the main responsible for the code they were approving. They were expected to test locally and should actively hunt for potential issues, such as checking in the logs that the ORM was building correct SQL. In my current team, the developer is the main responsible for the cod…
>This should be something agreed within a team, so that review standards are consistent across team members. Why? it feels like individual preference >such as checking in the logs that the ORM was building correct SQL. Couldnt the person who creates PR copy/paste sample generated SQLs into PR?
Code review isn't really something we do at an individual level because we feel like it, it's something the team, as an entity, does to ensure work is completely to a certain standard of quality. One's personal preference on reviews (or how to perform them) is less important than the team as a whole being comfortable with the process.
To your second point, sure, if that's what the team agrees is better.
Re: Ask HN: Do you pull and run code as part of code review?
#47Yes, I read the code and also run the code locally. Reading the code is for strategic larger picture thinking and running the code is to find tactical errors in the implementation.
Re: Ask HN: Do you pull and run code as part of code review?
#48this really depends on many factors like maturity of the tests and static code analysis, size and seniority distribution of the team and also if part of the pr acceptance is accepting the feature or if the team has a proper product management where the feature is accepted independent from the more code focused PR. i would totally agree with many comments, that a project that is way past mvp phase with proper feature acceptance and integration tests as part of the CI/CD pipeline should not require you to run the code for pr review, there are many phases in early projects where this might be necessary. eg when i review prs from junior devs where something just does not feel right, but change requests are too complex for simple comments on the pr i always use codespaces to run and manually test the code and understand what exactly they did and how things should be. also for a complex and critical feature i could not understand the PR just by reading the diff, its sometimes necessary to run and see the code in context to really understand it.
Re: Ask HN: Do you pull and run code as part of code review?
#49Very rarely. We have presubmit actions that confirm everything is building and passing tests. And I insist on at least some degree of testing in all cases. Running it locally would be of little help for our case because the systems I work on are large enough that it's not trivial to examine behavior through local commands.
Think of an rdbms, and I'm reviewing changes that optimize or allow new schema options. It would not be productive for me to design new schemas each time I review code. Instead, the person sending the review should already have done that work in tests.
Re: Ask HN: Do you pull and run code as part of code review?
#50Yes, absolutely. One time, my team was left without QA personnel, so we had to check features manually for a while. Even after this no longer became mandatory, running the code almost always reveals aspects worthy of a discussion as a part of code review. Obviously, this has a drawback of longer reviews, especially if testing a particular scenario involves quite a bit of effort. Would recommend, certainly beats longer feedback loop when the same issues are revealed during the QA acceptance phase.