Programming

568 readers

1 users here now

All things programming and coding related. Subcommunity of Technology.

This community's icon was made by Aaron Schneider, under the CC-BY-NC-SA 4.0 license.

founded 2 years ago

MODERATORS

[email protected]

What does your merge/code review process look like? (beehaw.org)

submitted 1 year ago by [email protected] to c/[email protected]

29 comments fedilink hide all child comments

Hey all! My team at work is struggling with growing pains of getting into a formalized review process, so I was wondering if any of you guys have some things to live or die by in your code reviews. How much of it is manual, or how much is just static code analysis + style guide stuff, etc?

you are viewing a single comment's thread
view the rest of the comments

[–] [email protected] 6 points 1 year ago (2 children)

We've got 20 or so devs and some infrequent contributors commiting to a pair of mono-repos, with some extra steps between them.

Our process looks like this:

develop on a feature branch
get two or more reviewers, sometimes devs that you've been talking with about the design, but if you don't know who we have a list of devs for different product areas.
only our newest stuff has auto-linting, otherwise style and static code analysis is all manual, but we're trying to automate as we go
need at least one approval to merge, not by any got rules, just by convention

All the code reviews are asynchronous, we're a distributed team so we don't like sit down in a room to talk about it, just comments on the PR.

Sometimes however you find a fix so small, you just commit and push to master. I'm not really in favor of that, but it happens.

[–] [email protected] 2 points 1 year ago

this is almost exactly how my company does it as well

except merges are typically handled by specific people and they only reach out for reviews if they aren't sure of how to handle a potential conflict

[–] [email protected] 1 points 1 year ago* (last edited 1 year ago) (1 children)

Midwestern b tier company:

Test your code on its own branch. Make sure it's not fucked
Pr to dev, do code review with devs that call you out on your bullshit (were all lazy sometimes)
Whip up QA doc, send the ticket to the QA queue
Confirm with BU that all their bases are covered and nothing is missing
Repeat steps for inevitable wishy washy scope creep from BA who wants to have inevitable meetings that could be done in emails
Complete merge to dev, merge dev to master, and tell devops that it's ready to go

[–] [email protected] 1 points 1 year ago

Ah yes, sounds about right. I particularly prefer the "make sure it's not fucked" step, very effective😂 I'd like to get more formal code reviews in place with my current team, I think we could all benefit.