5 ms·

I also worked at a place where we did OTS reviews. In my experience it was really good and it was easier getting deeper/more thorough feedback, since you're bas

by arnorhs 11y ago

I also worked at a place where we did OTS reviews. In my experience it was really good and it was easier getting deeper/more thorough feedback, since you're basically forced to tell what your code is doing etc. I'm really curious about how you feel people review more thoroughly when doing an async CR? Do you think there was an issue with people not feeling comfortable criticizing in person, or feeling like they'll just say "go on" even when they don't understand a code block, just b/c they didn't want to feel unproductive having the other person wait or something like that?

mahyarm 11y ago
It was just a trend I noticed, or maybe a difference in cultures. I notice my coworkers sometimes spending 15-20 minutes going over my code sometimes in phab. While at my past place they didn't dive deep into the code. People also don't feel as comfortable bringing up pedantic crap in person as much as they do over a web interface as you said. I think with OTS your going at the pace of the presenter, while with an async code diff, the reviewer is going at their own pace, can hop back and forth in between things and so on. It's also hard to keep track of things you have to change with an OTS, while with async code reviews the request list is right there already.
- sethammons 11y ago
  Sometimes we do group or individual OTS style CRs (new term for me, "OTS"), but with a twist. The reviewer walks through and describes the code, not the one who wrote it.
optforfon 11y ago
I had a bit of a problem with OTS reviews b/c not everyone is as professional when it's not in an open forum. We used GitLab merge requests with much better results. It's kinda hard to describe in words - but programming is an opinionated profession and people have a bad habit of telling you that you should rewrite stuff b/c of code-style differences and what in essence is non-substantive feedback. They want everyone to look like the way they would write it. Not to say it's always without merit.. but it's honestly kinda irritating after a while b/c you feel you are being micromanaged and are losing ownership of your work (when reviewed by someone who is senior to you)
- Can_Not 11y ago
  You should be coding in the official company coding style--or one should be written one for you to follow. In my experience supervising junior devs, reading code is like driving down a highway. When you submit code that is written out of sync with 99% of the entire company codebase, it's like going 60mph then suddenly the highway becomes a bumpy rocky dirt field. It's not really an opinion thing, it's a consistency thing. The time for opinions ended a long time if you are not the first programmer. You wouldn't join a law firm and start ignoring their legal document guidelines, because their senior lawyers need to read your written documents at the same fast speed they read everyone else's. We can spot your real mistakes a lot faster if you stop cluttering the codebase with the numerous cosmetic ones. Your seniors are pissed because you are paving a dirt road for them to drive on. You shouldn't be upset that they are asking you to do the bare minimum. If they don't have a formal style guide though, then they deserve the dirt road.
  - arnorhs 11y ago
    well said

by arnorhs 11y ago

mahyarm 11y ago

It was just a trend I noticed, or maybe a difference in cultures. I notice my coworkers sometimes spending 15-20 minutes going over my code sometimes in phab. While at my past place they didn't dive deep into the code. People also don't feel as comfortable bringing up pedantic crap in person as much as they do over a web interface as you said. I think with OTS your going at the pace of the presenter, while with an async code diff, the reviewer is going at their own pace, can hop back and forth in between things and so on. It's also hard to keep track of things you have to change with an OTS, while with async code reviews the request list is right there already.

sethammons 11y ago
Sometimes we do group or individual OTS style CRs (new term for me, "OTS"), but with a twist. The reviewer walks through and describes the code, not the one who wrote it.

optforfon 11y ago

I had a bit of a problem with OTS reviews b/c not everyone is as professional when it's not in an open forum. We used GitLab merge requests with much better results. It's kinda hard to describe in words - but programming is an opinionated profession and people have a bad habit of telling you that you should rewrite stuff b/c of code-style differences and what in essence is non-substantive feedback. They want everyone to look like the way they would write it. Not to say it's always without merit.. but it's honestly kinda irritating after a while b/c you feel you are being micromanaged and are losing ownership of your work (when reviewed by someone who is senior to you)

Can_Not 11y ago
You should be coding in the official company coding style--or one should be written one for you to follow. In my experience supervising junior devs, reading code is like driving down a highway. When you submit code that is written out of sync with 99% of the entire company codebase, it's like going 60mph then suddenly the highway becomes a bumpy rocky dirt field. It's not really an opinion thing, it's a consistency thing. The time for opinions ended a long time if you are not the first programmer. You wouldn't join a law firm and start ignoring their legal document guidelines, because their senior lawyers need to read your written documents at the same fast speed they read everyone else's. We can spot your real mistakes a lot faster if you stop cluttering the codebase with the numerous cosmetic ones. Your seniors are pissed because you are paving a dirt road for them to drive on. You shouldn't be upset that they are asking you to do the bare minimum. If they don't have a formal style guide though, then they deserve the dirt road.
- arnorhs 11y ago
  well said