this post was submitted on 08 Jun 2023
20 points (100.0% liked)

Programming

13389 readers
33 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
 

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
[–] alehel@beehaw.org 2 points 1 year ago (2 children)

I work on an old monolithic system (20 years ish). It's a case management system. We've been through many iterations of our workflow over the years I've worked there. Our current workflow is,

  • Create a branch from main
  • Create a PR against main when ready to merge
  • Every night a new build of the program is automatically built and pushed to a testing environment
  • Every Wednesday night we deploy a new version. We deploy the version that has been fully testet. So for instance, if we built v 9.101 on Tuesday night, but that build still has untested features, we deploy v 9.100 which was built Monday night, etc.
  • If there is a major bug in production, we
    • temporarily deactivate merging.
    • fix the issue in main.
    • build a new version, push it out to a test environment
    • our product owners test the fix along with other new features that have found their way into the build.
    • Once testet ok, we decide if the fix is important enough to take the system down during working hours, or if it can wait for a night-time deploy.
    • We open up merging again.

The obvious down side to this way of working is that the product owners might have quite a few features to test if there is a hotfix, and other features have been merged since deploy. This has almost never been an issue though. We almost always deploy the very latest version on Wednesdays, and if there is a major issue, it's usually discovered and reported before 11 am on Thursday. So the number of other features which have found their way into the code is usually 1 or 2 at the most. Each feature is usually quite small in scope, so it's actually worked quite well for us.

[–] pattern@beehaw.org 1 points 1 year ago (1 children)

How many developers do you have that you have to disable merging? Or is it more a safety-net?

[–] alehel@beehaw.org 2 points 1 year ago

Purely a safety net to avoid mistakes.

[–] mustyOrange@beehaw.org 1 points 1 year ago (1 children)

Merging straight from feature -> master is gutsy

[–] alehel@beehaw.org 1 points 1 year ago

Well. I told a slight lie. We go against develop. However, builds are created from develop. So we don't actually use main anymore. We used to, but our workflow changed over time untill we got to the point were we didn't use main anymore. I'm not even sure what we would use it for if we'd tried.