I don't have any particular rule for it, it depends entirely on the PR. If I feel confident that I can review the change without running it locally, I don't do it. But if I feel that my review might be compromised by not seeing the behaviour first hand or that I need to step through some part of the code myself to properly understand it, I'll run it.
Ask HN: Do you pull and run code as part of code review?
131–140 of 146 posts
Re: Ask HN: Do you pull and run code as part of code review?
#132I only do it if I suspect that the code will not work. Sometimes I can think of edge cases that I suspect are not covered; so I check out the code, add the edge case as a unit test, and if it fails I mention it in the code review for the other dev to add and fix. Other than that, no. I am reviewing the coding standard, architecture, and algorithms used. It's up to the dev to ensure that the code works and runs. The C…
Wouldn't it be better to ask in the MR if they could just write a test to cover that case? It prevents wasting your time writing a test case that's going to be thrown out and prevents the other engineer from being blind sided by someone else committing a test case to their branch. I'm really not sold that the approach you're taking is of more value than just asking the MR author to add the test themselves.
That's a good point and you are right. I have simply asked for the engineer to write the test in the past, but sometimes my curiosity just gets the better of me and I want to try it out myself. Other times I am not quite sure what is wrong but it just "feels" wrong so I end up throwing a couple unit tests at it to see if my intuition was correct.
Note that this is quite rare and is maybe a once in a 100 code review occurrence. Usually it's quite obvious that something is wrong and you can just point it out or start a discussion about it.
Re: Ask HN: Do you pull and run code as part of code review?
#133Isnt this what a ci/cd is supposed to do? Automatically?
Re: Ask HN: Do you pull and run code as part of code review?
#134[deleted]
Re: Ask HN: Do you pull and run code as part of code review?
#135Earlier quoted context omitted.
The fact that people think this is an excellent idea.....screen recording the proof that the feature works wtf what is the alternative you are getting? People are submitting features that don't work? Just fire them or teach them your set of standards. If my boss asked me to do this I would laugh and find a new job. I truly feel sorry for you but also please stop ruining our industry with this nonsense.
Requiring screen recording isn't about lack of trust or lack of competence. Rather it's a way of eliminating information asymmetry between the author and the reviewer: - The author might have checked that the code works fine (A) or haven't done it (B). - The reviewer might decide to run the code locally (1) or not (2). Combination A1 results in duplicate effort. Combinations A2 and B1 are perfect. Combination B2 resu…
All you have to do in a code review is review the code. You do not have to run the code nor should you be unless it is a very tiny project. It is the developers responsibility to test the functionality and get it ready for the QA process. If there is a dedicated QA that can test their functionality then the QA is responsible for testing on that front. The code reviewer is not supposed to be testing the feature works as intended for product. The manager of the team or product or client or stakeholder would be the ones testing yhe functionality and giving it a go ahead. It is not the job of the code reviewer to do that. It means you have not taught your developers the set of standards and practices to follow. If you have to get a screen recording of a feature working (probbaly in local or dev so its as useful as a unit test) then you or whoever is responsible has failed at creating a proper software development process
Re: Ask HN: Do you pull and run code as part of code review?
#136Earlier quoted context omitted.
The fact that people think this is an excellent idea.....screen recording the proof that the feature works wtf what is the alternative you are getting? People are submitting features that don't work? Just fire them or teach them your set of standards. If my boss asked me to do this I would laugh and find a new job. I truly feel sorry for you but also please stop ruining our industry with this nonsense.
Consider yourself lucky to be on a team where people are competent at testing the features they create.
Lucky? Im unlucky. Where tf are these jobs where you get away with submitting code that doesn’t work. The amount of process and procedures and standards we have to follow is a lot and it is exhausting. Need to find a job where i can get away with trash work
Re: Ask HN: Do you pull and run code as part of code review?
#137Earlier quoted context omitted.
Requiring screen recording isn't about lack of trust or lack of competence. Rather it's a way of eliminating information asymmetry between the author and the reviewer: - The author might have checked that the code works fine (A) or haven't done it (B). - The reviewer might decide to run the code locally (1) or not (2). Combination A1 results in duplicate effort. Combinations A2 and B1 are perfect. Combination B2 resu…
All you have to do in a code review is review the code. You do not have to run the code nor should you be unless it is a very tiny project. It is the developers responsibility to test the functionality and get it ready for the QA process. If there is a dedicated QA that can test their functionality then the QA is responsible for testing on that front. The code reviewer is not supposed to be testing the feature works…
You are 100% right and I agree with you completely.
I have failed to create a proper software development process.
Re: Ask HN: Do you pull and run code as part of code review?
#138Earlier quoted context omitted.
All you have to do in a code review is review the code. You do not have to run the code nor should you be unless it is a very tiny project. It is the developers responsibility to test the functionality and get it ready for the QA process. If there is a dedicated QA that can test their functionality then the QA is responsible for testing on that front. The code reviewer is not supposed to be testing the feature works…
You are 100% right and I agree with you completely. I have failed to create a proper software development process.
Well I apologize if I came off as harsh. I don’t mean to say you specifically. I just adamantly believe that we need to work towards process that allow you to trust the developer. Every situation is unique however and I cannot speak on all but I do think screen recording proof of work is not useful. Also if people are forced to screen record there us no scalability in projects software development
Re: Ask HN: Do you pull and run code as part of code review?
#139Earlier 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.
What's the point of checking for those if the bar for fully functional isn't met?
Re: Ask HN: Do you pull and run code as part of code review?
#140Earlier quoted context omitted.
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.
What's the point of checking for those if the bar for fully functional isn't met?
None. So don't do the review until CI passes.