Skip to content

Reduce the responsibilities of ComponentVersions

Robert Speicher requested to merge rs-component-versions-cleanup into master

Addresses technical debt outlined in https://gitlab.com/gitlab-org/release-tools/-/issues/389 for the ComponentVersions class, such that the class will better adhere to the Single Responsibility Principle, and its functionality will better match its name, i.e., it gets the component versions in gitlab-rails, with methods for returning them in formats that Omnibus and CNG work with.

For both Builder::Omnibus and Builder::CNGImage, the refactoring roughly followed this process:

  1. Move the methods exactly as-is from ComponentVersions, along with specs

  2. Make the methods instance methods rather than class methods

  3. Remove the type-specific naming (e.g., update_cng became just update)

  4. Remove the branch argument, relying on the internal instance variable provided to the class's initializer

  5. Remove the version_map argument, relying on the internal attr_reader and the value assigned during execute

  6. Remove the update_* method as it was basically a wrapper for this very simple logic that was moved directly into the execute method:

    if changes?
      commit
    else
      # log
    end

The CNG-related logic saw additional refactoring because we wound up fetching and parsing the YAML variable definitions in two places that wasn't necessary to begin with (see here and here).

All of this greatly reduced the number of necessary specs, due to making the "helper" methods internal private methods. Because all we need to end up testing is:

  1. Does the class fetch the versions from ComponentVersions formatted for its respective builder?
  2. When those versions are identical to what's defined in the builder, do we do nothing?
  3. When those versions differ from what's defined in the builder, do we commit the changes?

So while this MR shows a bunch of deleted tests, we're still covering the same amount of functionality.

Edited by Robert Speicher

Merge request reports