Dev 1: "I'm just going to reword this error message because I think it sounds awkward. Harmless, I'll ship it"
Dev 2: "Actually you've removed a keyword that our error messaging framework uses to highlight the error in neon red. We've seen in a/b testing that this bright color prevents error blindness 65% of the time, and saves users from having to fix mistakes that cost them on average 1 hour of confusion"
Changing strings are about the most harmful thing to the bottom line where I work. :)
However, in your case, that should probably be automated so people can’t do that just like we helpfully provide similar already translated strings when you add new strings or change them. People shouldn’t be able to break production, review or no.
That's the whole point of continuous integration, and that's what the author is trying to push. That you actually move faster by letting people push and fix bugs versus push a change, wait some time for a code review (somewhere 1min...1day), go back and forth on some style things, maybe a the reviewer managed to win the game of "spot the bug," and then ship, and then fix any delinquent bugs.
Like, unless if that middle reviewer is doing something that really can't be done elsewhere, it just serves to slow things down.
Hmm, so something is parsing a string looking for specific character sequences? I'd say the developer who changed that is doing the team a favor by surfacing a terrible practice.
Edit: NVM I see you acknowledged that in another comment.