4 ms·
No, I mean like the primary function of every PR is to ensure the code does what its trying to do and doesn't (for example) fail to compile, or break the build
by Quarrelsome 5y ago
No, I mean like the primary function of every PR is to ensure the code does what its trying to do and doesn't (for example) fail to compile, or break the build or create an infinite loop or an out of memory exception.
- digisign 5y agoArguably too fundamental to mention, at least in one slide. Perhaps covered during the developer-testing period, before peer review.
- deleted 5y ago[deleted]
- b3morales 5y agoOn a dayjob closed-team project, if my coworkers are putting up PRs without making sure they meet functional requirements, that is a problem that needs to be solved several steps before the PR. If it's a junior person who doesn't know better, more working side-by-side with them to get to the PR. If it's a senior person dealing with a shitty spec, more discussion with PMs/designers before we start coding. If it's a senior person who's putting up broken code because they don't care...that's probably a management conversation. I have to be able to trust that my teammates are doing their best to produce working components. Otherwise code review is a bandaid covering a hole in the hull of a sinking ship.
- deleted 5y ago[deleted]
- icedchai 5y agoIf code is getting to the PR stage and doesn't compile, you have serious organizational problems. In previous jobs, I have "reviewed" code that has clearly never been run (syntax error in a script) and it makes me question my own sanity.
- Quarrelsome 5y agoOr breaks the build in an otherwise non-obvious fashion or fucks the installer?
- icedchai 5y agoOne of the first things I check on a PR is if it passed CI cleanly. If the build has failed (failing tests, build problems, whatever), I get annoyed I was even asked to look at it.