I'm going to differ on the 'don't be a blocker' thing.
It varies by team. If your whole team agrees that the code reviewer is the 'gate' then people understand that they just need to fix the thing and move on. I personally love that style of working because you tend not to write trivial suggestions and everyone knows that what people have written isn't meant as nit-picky and just has to be changed.
If you don't think it's the right change then the other person can just ask why it's important and hopefully get educated. I also tend to just trust my colleagues enough that what they suggest is a good change. I often find I may miss small things through like lack of documentation or whatever (get tired after a while) and those reminders in the CR are the equivalent of a spotter getting you to do one last rep at the gym.
It's a problem for small teams and people with little experience with code reviews. The 'walking on egg shells' thing happens ALL the time.
I've never seen this get solved overnight. Your management really needs to sit down and explain their expectations around code reviews - that fixing things based on co-worker suggestions is a positive thing, that having negative comments won't affect their performance reviews, etc.
I also find that with people like that you really do need to let more things go. If it's not a critical bug or an actual problem with the code then let it slide. Some people just cannot take criticism (even constructive) and the bad blood you'll get by persisting with it will not help you.
I also tend to find these developers are not necessarily the strongest devs and often have other personality problems. This likely indicates that your company needs to focus a lot more on the hiring and culture.
Yup - it's also similar to how great developers don't debug or code by braille (did that work? nope. How about this? Nope. Ok lemme try this...) but rather will sit and think through a problem.
#3 is where a lot of tension can arise during code reviews. Some people, usually those who aren't familiar with the process, will wonder why you are taking so long to review the code or why you are asking questions that are 'out of scope'.
I find that larger organizations are often more competent at code reviews. In the startup world #3 is a rare, rare thing.
This is exactly my feeling too. Justin seems too smart by half.
His attitude is very much like an ivory tower academic who is befuddled that people don't follow best practices.
I also get the feeling he's not used to having to admit he's wrong. I guess you don't make it to 'head of security' at Google by having a little humility but his responses are really not very encouraging.
Yeah but his point is really silly. He's trying to make the argument that by making it impossible to lock the car we won't leave valuables in it.
His argument is that since a person could just smash the windows and open the car that way there's no point in putting locks on the door.
It's just a very myopic and weirdly out of touch position. He seems to think that he's 'training' users to have more secure practices? This isn't the business world. You don't get to blame the user for not being security experts. He should be doing everything in his power to make it inconvenient and difficult to access a user's passwords.
The value, I think, is in challenging your own beliefs.
To be honest this reminds me a lot of how Microsoft used to treat issues in their code/software 'Oh, that's a user error. That's not a bug, that's a feature!'. And then when you get pushback you go 'I've discussed this enough, no more talking with the plebes'.
Your axiom seems to be that anyone with access to your computer should be 'trusted'.
In other words, if I hand my laptop to my spouse I am essentially granting her root privileges.
A lot of us are making the point that this isn't true. I may have a wife, children or a roommate who I trust to use my laptop but don't want to make my passwords easily visible.
When I hand my laptop to my wife I have an expectation that without resorting to some special tools she should not be able to find out what my Amazon password is or what my hotmail password is.
Your position seems to be that by making these passwords visible you are encouraging more secure behavior - ie. I will now log my computer into a 'guest' account every time I give it to my wife.
It just seems like you don't get how people ACTUALLY use your product. For many reasons I'm not going to lock my computer every time I give my laptop to my wife or a roommate. I have an expectation that there is SOME obscurity that protects my passwords even if it's just obscurity by not explicitly showing the password. You're not going to change my behavior and frankly most of us are pretty shocked that a) you are so resistant to challenging your own axioms b) you think this is somehow our fault for expecting Chrome to not have a giant 'show passwords' button.
You need to challenge your assumption that the 'attacker' is some malicious agent. Widen the scope to also include the suspicious spouse or the prankster roommate and you'll understand why we think this is a bigger deal than you seem to consider it. Even if it just presents a small barrier I think most of us feel that small 'annoyance' is enough to prevent pranks and snooping spouses.
That's fair enough but then why put in a giant 'SHOW PASSWORD' button. Why not just remove that button.
I expect a hacker using my computer with admin rights to get everything. I don't expect my GF to get my passwords simply by pressing the 'show passwords' button.
And it's not very realistic for me to tell my GF she can't use my computer or to log out every time I give it to her. She's going to be kind of insulted if I do that. I just expect you guys to understand that in this day an age people DO actually share their computer with people without expecting them to have a giant 'SHOW PASSWORD' button in their favorite app.
You're thinking of security in the sense of some hacker or someone who has technical abilities.
What about the jealous ex-bf? He asked to use the gf's computer when they were dating, easily grabbed ALL of her password info and now she has to change everything when they break up. You're giving complete technophiles the ability to nab passwords. The question is WHY - what utility is there to make these show up in plain text over just prompting for a master password? What's the use case that you NEED to make these visible so easily?
It's also a terrible, terrible excuse to say 'well, there are other ways to get that info so our security flaw isn't an issue since it's already trivial'.
The fact of the matter is that you should at the very least require the master password to make these other passwords visible. There should be SOME authentication being done here.
What would this conversation be like if we were talking about gmail.com? You think it'd be OK to show in plan text a person's gmail password in the Settings page. I mean, if you logged in then of course you are the only person who should be looking at it.
It varies by team. If your whole team agrees that the code reviewer is the 'gate' then people understand that they just need to fix the thing and move on. I personally love that style of working because you tend not to write trivial suggestions and everyone knows that what people have written isn't meant as nit-picky and just has to be changed.
If you don't think it's the right change then the other person can just ask why it's important and hopefully get educated. I also tend to just trust my colleagues enough that what they suggest is a good change. I often find I may miss small things through like lack of documentation or whatever (get tired after a while) and those reminders in the CR are the equivalent of a spotter getting you to do one last rep at the gym.