4 ms·

Yeah, I'm in a similar situation at the moment. It's incredibly frustrating because during code reviews I will request changes so it's not such a broken hack j

by jack_h 6y ago

Yeah, I'm in a similar situation at the moment. It's incredibly frustrating because during code reviews I will request changes so it's not such a broken hack job, and the response will basically be "No, it's not worth changing". At which point I'm the one "holding up development". We wasted hundreds of development hours during the last project because of this persons "inventive" code, and nobody seems to understand what's going on. Shame the job market is a bit crap right now.

munk-a 6y ago
It's hard to get more strength to push back with out of thin air. I'd encourage you to try pushing for more detailed post-mortems (if you don't already have them) and just keep an eye out on how much curtailed reviews cost the company. You also really want an advocate for code maintenance and if you don't have one of these with a loud voice there isn't a really feasible way to solve it except becoming it yourself and earning the trust of those above you. Two pieces of actual useful advice I can offer are: 1. A review style I picked up based off of RFC 2119[1] basically the reviewing software we use allows us to mark particular comments as blocking of non-blocking and I pair that with the usage of MAY/SHOULD/MUST within the comment language i.e. "We're using the old `array()` syntax here instead of `[]` we MAY wish to use the more modern syntax" this allows me some room to elevate necessary change while keeping in the nitpicks I really want to throw in (and I do try and minimize them) without lowering the power of the strong comments. I've used MUST maybe three times always for something incredibly terrible like pages not loading or migrations to the DB that are unsafe and cause data loss. 2. Agree on syntax and style rules and enforce them. It's easier to get people to agree to rules once than try and argue for them on each PR - anything like brace placement or line limit shouldn't come up repeatedly since it wastes everyone time and makes folks feel belittled. 1. https://www.ietf.org/rfc/rfc2119.txt https://www.ietf.org/rfc/rfc2119.txt
- gen220 6y ago
  This is great advice. Just have some minor thoughts to tack on. Post-mortems are great for many reasons. For the case of GP, one particular advantage is that they align senior peoples' understanding: we shouldn't do X again. If you have a strong narrative for why a project failed, post-mortems are a formal setting in which you can present this narrative with concrete evidence to higher-ups. In the future, when you see warning signs that a mistake is approaching repetition, you can raise the concern up the chain, invoking the memory of the post-mortem to motivate their intervention. I also totally agree that a sincere and high-quality code review process is required for high quality code. Your 2119 recommendation is excellent. I'd also recommend doing some reading on commit message templates that smart people follow, they've improved my commit game, big-time.

by jack_h 6y ago

munk-a 6y ago

It's hard to get more strength to push back with out of thin air. I'd encourage you to try pushing for more detailed post-mortems (if you don't already have them) and just keep an eye out on how much curtailed reviews cost the company. You also really want an advocate for code maintenance and if you don't have one of these with a loud voice there isn't a really feasible way to solve it except becoming it yourself and earning the trust of those above you. Two pieces of actual useful advice I can offer are: 1. A review style I picked up based off of RFC 2119[1] basically the reviewing software we use allows us to mark particular comments as blocking of non-blocking and I pair that with the usage of MAY/SHOULD/MUST within the comment language i.e. "We're using the old `array()` syntax here instead of `[]` we MAY wish to use the more modern syntax" this allows me some room to elevate necessary change while keeping in the nitpicks I really want to throw in (and I do try and minimize them) without lowering the power of the strong comments. I've used MUST maybe three times always for something incredibly terrible like pages not loading or migrations to the DB that are unsafe and cause data loss. 2. Agree on syntax and style rules and enforce them. It's easier to get people to agree to rules once than try and argue for them on each PR - anything like brace placement or line limit shouldn't come up repeatedly since it wastes everyone time and makes folks feel belittled. 1. https://www.ietf.org/rfc/rfc2119.txt https://www.ietf.org/rfc/rfc2119.txt

gen220 6y ago
This is great advice. Just have some minor thoughts to tack on. Post-mortems are great for many reasons. For the case of GP, one particular advantage is that they align senior peoples' understanding: we shouldn't do X again. If you have a strong narrative for why a project failed, post-mortems are a formal setting in which you can present this narrative with concrete evidence to higher-ups. In the future, when you see warning signs that a mistake is approaching repetition, you can raise the concern up the chain, invoking the memory of the post-mortem to motivate their intervention. I also totally agree that a sincere and high-quality code review process is required for high quality code. Your 2119 recommendation is excellent. I'd also recommend doing some reading on commit message templates that smart people follow, they've improved my commit game, big-time.