Ask HN: Reviewing PRs in a language you don't know?

3 points by Steppschuh 8 years ago | 3 comments

I'm an Android engineer with only basic knowledge of a few languages that are not Java. I'm responsible for a project that will now also include an iOS app. I have never worked with Swift before, but of course want to make sure that the new iOS developers are building up a healthy codebase.

I feel like I'll only be able to look at a higher level when reviewing their work (like architecture, patterns, etc.). I'm considering taking some Swift courses to get familiar with the language.

I'm looking for tips or experiences how to deal with this situation.

blowski 8 years ago |

I have to do this a fair bit. My background is PHP, but as a lead I often find myself reviewing JavaScript, and config files for CI/CD that I don't entirely understand. While I understand the basic syntax, there are a lot of functions and patterns that are alien to me.

So I focus most of my attention on test cases. Do they cover all the edge cases we discussed in sprint planning? Now that the implementation is done, can I see any more that area necessary? How clear are the test cases?

Then I turn to the general readability of the code. Are there any strange variable names? Any complicated blocks that could be made clearer? Any strange bits of magic that could have a comment? I might also look for other common anti-patterns - duplicated code, magic numbers, dead code.

Some of this depends on the idiomatic use of the language, and the code style the team has agreed on. There will be places where you'll be wrong, and the developers will be doing the right thing, despite it looking strange to you.

But pushing people to explain their decisions can make them better developers, as they'll have to think about why they code in that way instead of "I dunno, I just did".

rococode 8 years ago |

I would be very hesitant to be the only person signing off on the PRs. Maybe if your company has multiple people approving PRs. If you have no other choice, just try to make it clear to your bosses that you dont actually know the language so you can't guarantee that your reviews will be as rigorous as they would be in Java.

In my experience, it might actually be harder to notice high-level than low-level, because code patterns are kind of tied to languages. Some of the theory stays the same like factories or whatever, but the language-specific things tend to vary a lot.

Take Java, for example. If someone uses an enum or a public static variable or static class with static variables or whatever, those are kind of design decisions that might arguably have a "right" answer to an experienced Java coder. But a newer Java coder probably wouldn't really be able to readily distinguish between use cases, only say that they are correct and somewhat neat code. It's much easier to catch weird syntax or bad style than to say "ok but you probably should restructure these couple of files" in a language you're not super familiar with.

Anyways I kinda rambled but the tl;dr is 1) let your bosses know (to the extent that you're comfortable with) that it's not your primary language, and 2) it's fine to learn as you go, just be aware that there's stuff to learn

klez 8 years ago |

It's weird that you have to be the one that reviews code when you don't know the language it's written in.

Is this a small organization or are there any staffing limits?

One possibility would be to take a Swift coder you trust from your team and have him/her do the review for you.