Skip to content

Fix remaining cop offenses for Gitlab/NamespacedClass

Problem

In Create new models / classes within a module / n... (#212156 - closed), it was decided that classes should have namespaces.

In Create Cop to enforce the use of namespaced cla... (!51236 - merged) we introduced a cop Gitlab/NamespacedClass that enforces each class to have a namespace.

There is a list of offenses that need to be fixed. This issue is to track the progress.

There are 2 main groups of TODOs that we can distinguish:

  1. Files that we know where to be moved to - Either there is already a namespace for it or it's clear what new namespace they will go in. Please refer to our guide on namespaces.
  2. Files that are either difficult to move or the namespace isn't straight forward to identify. For those files let's have a preliminary discussion in this issue and we'll create more specific issues as we identify next steps. For example: User, Project, Repository, which namespace (representing the bounded context) should they go?

Solution for the 1st group

  1. pick one or more files from the list
  2. create a branch
  3. delete the file from .rubocop_manual_todo.yml
  4. move the file under the right namespace. Create a new one if it doesn't exist yet.
  5. commit and create a merge request with static code analysis label
  6. link the MR to this issue
  7. 🎉

Solution for the 2nd group

  1. Create a discussion thread with a header indicating the class/namespace to discuss - E.g. ## User class
  2. let's have a discussion where these should go, what bounded context we should have, and what other classes belong to the bounded context Let's discuss in this issue

Difficulties defining Domain Boundaries and Bounded Contexts

We should certainly try to fix all remaining errors for this cop, as tracked in this issue.

However, the nature of this cop means that it's usually not something you want to fix as part of an unrelated MR. Adding a namespace to a model or lib file means updating any other dependencies which may refer to the class by name, and the possibility of introducing regressions if you miss anything.

Furthermore, even if you want to fix it, there is currently much ambiguity around defining clear domain boundaries and bounded contexts. See the above issue and this documentation: https://docs.gitlab.com/ee/development/directory_structure.html#use-namespaces-to-define-bounded-contexts

See also the (many!) comments on the following issue, which has a lot of in-depth discussion and good points regarding the current difficulties in defining domain boundaries: Proposal: split GitLab monolith into components (#365293 - closed)

So, it's often unclear what namespace is appropriate to use (or create), even if you did want to take the time to attempt a fix.

(Note from @cwoolley-gitlab: my comment here contains a discussion and several references making the case that, based on objective criteria, the GitLab Rails app is not currently a well-structured, modular monolith)

Edited by Chad Woolley