Skip to content

Use decorators and interface segregation to solve overgrowing models problem

Summary:

Using decorators to segregate interfaces and improve testability may be a good solution for overgrowing models - https://gitlab.com/gitlab-org/gitlab-ce/issues/13484#note_3972945

Original issue:

According to this discussion, we can try moving build implementation to separate concerns, which can have some benefits.

Example: https://gitlab.com/gitlab-org/gitlab-ce/blob/6536a51c9034e1260bf4139b41d4c69c3cf2cf00/app/models/ci/build/erasable.rb

Reasoning behind this:

  1. We have less implementation in a single file - it is easier to browse it
  1. We have semantically coherent implementation in separate files - it is easier to find elements related to each other, for a given feature (like implementation that supports build erase only)
  2. We can exercise concerns inside separate tests - it is easier to maintain main model spec suites
  3. We can easily remove a feature if we decide so
  4. In some case we prevent merge conflicts between CE -> EE, as EE only features can go to concerns and it is only one common line between CE and EE in a model, also specs become separate files
  5. If we decide that this implementation belongs somewhere else it is then easier to move it to different place
  6. We improve maintainability of our models
  7. We make contribution barrier lower, model with 1000+ is difficult to understand, and as long as we keep our concerns separated and coherent, it is easier to understand what is going on (however I think that using composition instead of mixins/inheritance would be better here, see commit comments above, mixins are more idiomatic)
  8. We are able to introduce a starting point for structure/architecture.

@ayufan, @DouweM, @rspeicher what do you think about it?

Edited by 🤖 GitLab Bot 🤖