Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Ask HN: Do you pull and run code as part of code review?

news.ycombinator.com

Post reply on HN
Print view View original

141–146 of 146 posts

Re: Ask HN: Do you pull and run code as part of code review?

#141

Post by magusd »

bradwood wrote: post #130
BossingAround wrote: post #89

Earlier quoted context omitted.

But you still have to test the actual functionality, don't you? What if the author makes a gif of clicking a button, but doesn't record if the browser back button still works? I'd think that for frontend, you'd have CI/CD pipeline that deploys the code into a staging server, where I can test it myself.

Code review != Testing. A lot of people conflate these IMHO. Code review should not really be about whether it "works" or not. That's what tests are for. Code reviews are about checking for code complexity, good use of abstractions, readability, etc.

this should be the most upvoted comment

Re: Ask HN: Do you pull and run code as part of code review?

#142

Post by presentation »

bradwood wrote: post #130
BossingAround wrote: post #89

Earlier quoted context omitted.

But you still have to test the actual functionality, don't you? What if the author makes a gif of clicking a button, but doesn't record if the browser back button still works? I'd think that for frontend, you'd have CI/CD pipeline that deploys the code into a staging server, where I can test it myself.

Code review != Testing. A lot of people conflate these IMHO. Code review should not really be about whether it "works" or not. That's what tests are for. Code reviews are about checking for code complexity, good use of abstractions, readability, etc.

And also to spread knowledge of aspects of the system across the team.

Re: Ask HN: Do you pull and run code as part of code review?

#143

Post by slaymaker1907 »

paradite wrote: post #25

I'm a senior frontend engineer doing a lot of code reviews and my team loves my MR (PR) submission standard: - Require the author to attach screenshots of the feature in a markdown table format showing the before and after comparison. This is enforced using a MR template. - Require the author to attach screen recording for complex user interactions. Overall effect is that it saves everyone's time. - The author has to…

Word is actually really great for this stuff. You get a portable file and can add notes to the screenshots. There is even a screenshot button built into Word!

For recordings, if you use Windows you can bring up the game bar with Win+G. It has a very easy to use screen capture tool meant for gaming, but perfectly fit for small software demos. It even captures sound which is sometimes useful.

Re: Ask HN: Do you pull and run code as part of code review?

#144

Post by tomatohs »

slaymaker1907 wrote: post #143
paradite wrote: post #25

I'm a senior frontend engineer doing a lot of code reviews and my team loves my MR (PR) submission standard: - Require the author to attach screenshots of the feature in a markdown table format showing the before and after comparison. This is enforced using a MR template. - Require the author to attach screen recording for complex user interactions. Overall effect is that it saves everyone's time. - The author has to…

Word is actually really great for this stuff. You get a portable file and can add notes to the screenshots. There is even a screenshot button built into Word! For recordings, if you use Windows you can bring up the game bar with Win+G. It has a very easy to use screen capture tool meant for gaming, but perfectly fit for small software demos. It even captures sound which is sometimes useful.

I'm working on an "instant replay for developers" desktop app for exactly this case. Create desktop instant-replays to embed in pull requests:

http://replayable.io/

Re: Ask HN: Do you pull and run code as part of code review?

#145

Post by epolanski »

bradwood wrote: post #130
BossingAround wrote: post #89

Earlier quoted context omitted.

But you still have to test the actual functionality, don't you? What if the author makes a gif of clicking a button, but doesn't record if the browser back button still works? I'd think that for frontend, you'd have CI/CD pipeline that deploys the code into a staging server, where I can test it myself.

Code review != Testing. A lot of people conflate these IMHO. Code review should not really be about whether it "works" or not. That's what tests are for. Code reviews are about checking for code complexity, good use of abstractions, readability, etc.

You have to review tests and you are back at square one.

Re: Ask HN: Do you pull and run code as part of code review?

#146

Post by jcubic »

In my last project at the current job, I've had one team member that always pulled the code and check it locally. And that from my experience was best code review that I've ever had. He sometimes found a bug in my code, because of something that I didn't tested, even before go to dev env and tested properly by QA.

No one else in the team was doing the same including myself. I'm not exactly sure but maybe part of the reason was that he was not much familiar with the code base, and checking if it works was the way to test if the code is correct.

Post reply on HN