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?

  • alehel@beehaw.org
    link
    fedilink
    arrow-up
    0
    ·
    2 years ago

    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.orgOP
      link
      fedilink
      arrow-up
      1
      ·
      2 years ago

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

      • alehel@beehaw.org
        link
        fedilink
        arrow-up
        1
        ·
        2 years 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.