I run data pipelines using some
Docker as a function pattern and if the ELT is small o kickoff to see if it works and then I start the review. Takes a bit more time but at the end of the day everyone has a bit more security to review a “compiled” piece of code for our ELT.
Ask HN: Do you pull and run code as part of code review?
71–80 of 146 posts
Re: Ask HN: Do you pull and run code as part of code review?
#72I'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…
This is an excellent idea for a submission standard, will have to store this in the back of my mind.
Re: Ask HN: Do you pull and run code as part of code review?
#73I expect automated tests to run, and where practical a demo link with the change to be provided for larger changes that aren't captured by automated testing.
Re: Ask HN: Do you pull and run code as part of code review?
#74Sometimes I do, esp. when CI is behind and doesn't have the latest compilers, sanitizers and not enough warnings turned on.
or as in the latest case when the developer has the newest msvc compiler with ' as digit separators, but the CI not.
Re: Ask HN: Do you pull and run code as part of code review?
#75I'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…
This is great, do you do this in github? Or elsewhere? Just curious if you had a template to hand that myself and others could use =)...
"MR" so probably Gitlab
Re: Ask HN: Do you pull and run code as part of code review?
#76Depending on the change and my understanding of it, I will: (a) review the changes, (b) browse the version of the files on GitHub, (c) checkout the version locally and browse in IDE, (d) run stuff (app, tests, console).
Re: Ask HN: Do you pull and run code as part of code review?
#77I've worked mostly on back-end stuff and "works for me/this" is asking for trouble. Making sure it builds is literally the minimum review you can do. You learn whether it actually compiles and whether it requires changes to the development environment.
Re: Ask HN: Do you pull and run code as part of code review?
#78Every time I skipped that part and the change wasn't trivial, I missed something important, so yes.
I'm a big proponent of "trial by fire" reviews. The idea is to setup a dumpster environment, deploy the change there and give it for a spin to someone who's keenly interested in seeing some progress - sometimes it's the PM, other times the sales guy - anyone who has some time in between meetings and would appreciate a little bit of variety in their work life.
Such people are amazingly talented at finding catastrophic edge cases.
Re: Ask HN: Do you pull and run code as part of code review?
#79Yes. I didn't used to, but that was a very big mistake. Just wait until something you signed off on doesn't run and folks ask, "But didn't you review this code and say that everything 'looked good'? It doesn't even run!" It's embarrassing to say the least. If you're doing a code review, run the darn code. What would you think of a mechanic who didn't make sure the car was able to turn on after claiming it was fixed?
My job isn't to make your code work, it is to find those little things that don't seem to be bad that are. I have tools for obvious things.
Which is to say 'it doesn't work' is nitpicking that reviewers shouldn't be wasting their time checking.
Re: Ask HN: Do you pull and run code as part of code review?
#80I never do, you have to trust people.