Changing how I review code
itnext.io
Changing how I review code
1–10 of 80 posts
Re: Changing how I review code
#2Re: Changing how I review code
#3https://archive.is/jeDmL
Can somebody post a link which wouldn't start with "One more step Please complete the security check to access"?
Re: Changing how I review code
#4> Another common critique is that the pull requests encourage reviewers to only look at the code. They never even pull down the changes they are reviewing!
You should run it somehow, but if you've got CI running tests, which I think you should have anyways, then does it matter?
Re: Changing how I review code
#5> Another common critique is that the pull requests encourage reviewers to only look at the code. They never even pull down the changes they are reviewing! You should run it somehow, but if you've got CI running tests, which I think you should have anyways, then does it matter?
At least with UI code, the tests / code don't really express the experience for the user. (But it's getting better with time!)
In that practice, it's better to run it locally! For each UI file / function, try to get it to execute from a user's POV.
Re: Changing how I review code
#6> Another common critique is that the pull requests encourage reviewers to only look at the code. They never even pull down the changes they are reviewing!
Our CI automatically deploys a "review app" when someone creates a pull request. This simplifies code review, QA and demoing!
Re: Changing how I review code
#7> Another common critique is that the pull requests encourage reviewers to only look at the code. They never even pull down the changes they are reviewing! Our CI automatically deploys a "review app" when someone creates a pull request. This simplifies code review, QA and demoing!
It sounds cool that you create on-demand review app. In a small scale it works. In a larger scale development is absolute impossible due to resource constraints to create on-demand environment for review app.
For instance, in a warehouse automation infrastructure that has 30 devs, working on 8 feature simulataneously, it is next to impossible to create review app with the resources they need.
Re: Changing how I review code
#8> Another common critique is that the pull requests encourage reviewers to only look at the code. They never even pull down the changes they are reviewing! You should run it somehow, but if you've got CI running tests, which I think you should have anyways, then does it matter?
That is what testers are for. Testing is great thing.
Re: Changing how I review code
#9I had the pleasure of being at a startup with a very talented SRE named M. Harrison. His minimum-level review criteria were that when you were planning to approve a PR, you should summarize everything the PR does in the approval comment. Responses like ":+1:" or "LGTM" were not allowed. When I would read a PR intending to understand and then summarize, the errors or omissions (eventually) started to just jump out at me. I'm not sure if I'm articulating it properly...but that expectation of a small book report-ish paragraph helped me become much better at critically reviewing PRs.
disclaimer: the majority(not all) PR's were in a very very large terraform codebase.
Re: Changing how I review code
#10> Another common critique is that the pull requests encourage reviewers to only look at the code. They never even pull down the changes they are reviewing! You should run it somehow, but if you've got CI running tests, which I think you should have anyways, then does it matter?
That is what testers are for. Testing is great thing.
/s/testers/users
Just kidding and in reality UI tests exists and aren't super difficult to set up. The front end has plenty of tools like Cypress and its pretty simple to automate running a bunch of tests that screenshot and diff compare your site.
I never really pulled down UI code to test them out. There has to be a certain level of trust between engineers. Sometimes I'd ask or post screenshots of changes but that was rare.