31–40 of 146 posts
Only if something really draws my attention do I pull and run a particular PR (CR, CL, whatever yo call it). Once I did that to illustrate that the change introduces a security issue, so I had to write an exploit against it, and demonstrate how it works. (It helped.)
1. PRs should target development, which is an *unstable* branch, so no biggie if something breaks
2. CI should catch things like syntax errors, failing tests, or poor test coverage
3. I trust my colleagues to do their work properly and fix things if they've made a mistakeIf it's code that is really changing behavior significantly then I'll run it on my device and just see if I notice anything. I've found some surprisingly gnarly bugs this way.
No. 1. PRs should target development, which is an *unstable* branch, so no biggie if something breaks 2. CI should catch things like syntax errors, failing tests, or poor test coverage 3. I trust my colleagues to do their work properly and fix things if they've made a mistake
- how can we even say it works?
- who’s responsibility is it to fix it?
- and when does it get fixed?
This 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…
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?
Your CI should be robust enough to catch "does it run" type issues. That's not always possible for one reason or another. If there are things that your CI can not currently check, you should be verifying local. You should also be striving to close those gaps in your CI.
Usually I don't, because the CI bot does it. I do read the code thoroughly though. Only if something really draws my attention do I pull and run a particular PR (CR, CL, whatever yo call it). Once I did that to illustrate that the change introduces a security issue, so I had to write an exploit against it, and demonstrate how it works. (It helped.)
Now that I’m in startup land, we use Heroku’s Review App feature, which is by far my favorite Heroku feature, and helps us catch bugs all the time
In those cases I’ll pull the code
No. 1. PRs should target development, which is an *unstable* branch, so no biggie if something breaks 2. CI should catch things like syntax errors, failing tests, or poor test coverage 3. I trust my colleagues to do their work properly and fix things if they've made a mistake
I don’t get your first point. If the pull request doesn’t run: - how can we even say it works? - who’s responsibility is it to fix it? - and when does it get fixed?