3 ms·

I don't understand why people hate CR. You're a professional you should be able to defend your code, and explain your reasoning. If you can't, you need to chan

I don't understand why people hate CR. You're a professional you should be able to defend your code, and explain your reasoning. If you can't, you need to change it, and you'll improve at the same time. I mean most changes in code review only take 5 minutes to rectify, so I often don't buy the time argument. If you getting bit architectural change problems in your code review, you obviously didn't collaborate with the team very well... People should roughly know what your approach is before the CR even arrives at their door

wrestlerman 8y ago
I do think that code changes take more than 5 minutes, why? As a person, who pushed the code: 1. I have to read the comments 2. Decide if they are right/it's worth a hassle 3. Sometimes you have to reach out to the author of the comment to clarify some things. 4. Change your branch 5. Change your (thinking) context 6. Apply stuff 7. Push stuff 8. Notify the person that it's done. Many times it's more than 5mins, more like 10/15mins+ I don't hate CR. Someone below explained it better than me, that different people see the code differently. And you should respect that someone wrote it that way and not try to force your way upon them. > If you getting bit architectural change problems in your code review, you obviously didn't collaborate with the team very well... People should roughly know what your approach is before the CR even arrives at their door I agree with it a lot. I think that would solve a lot of problems, but not necessarily the small ones.
JoeAltmaier 8y ago
Yes, but that's not what happens. Folks say "you tested for an empty pointer wrong" meaning not how they prefer. Or "This could be coded another way that I think is clearer". Here's a litmus test: if the comment has nothing to do with the correctness of the solution to the ticket, then don't comment. Instead write another ticket to address {whatever}. If it doesn't rise to the level of a ticket then guess what - you're being pedantic.

wrestlerman 8y ago

I do think that code changes take more than 5 minutes, why? As a person, who pushed the code: 1. I have to read the comments 2. Decide if they are right/it's worth a hassle 3. Sometimes you have to reach out to the author of the comment to clarify some things. 4. Change your branch 5. Change your (thinking) context 6. Apply stuff 7. Push stuff 8. Notify the person that it's done. Many times it's more than 5mins, more like 10/15mins+ I don't hate CR. Someone below explained it better than me, that different people see the code differently. And you should respect that someone wrote it that way and not try to force your way upon them. > If you getting bit architectural change problems in your code review, you obviously didn't collaborate with the team very well... People should roughly know what your approach is before the CR even arrives at their door I agree with it a lot. I think that would solve a lot of problems, but not necessarily the small ones.

JoeAltmaier 8y ago

Yes, but that's not what happens. Folks say "you tested for an empty pointer wrong" meaning not how they prefer. Or "This could be coded another way that I think is clearer". Here's a litmus test: if the comment has nothing to do with the correctness of the solution to the ticket, then don't comment. Instead write another ticket to address {whatever}. If it doesn't rise to the level of a ticket then guess what - you're being pedantic.