Skip to content

Ban rails default_scope usage

Nick Thomas requested to merge ban-rails-default-scope into master

Description of the proposal

This has come up twice in two days in the #backend-maintainers channel:

My opinion is that this AR method does more harm than good, especially in large codebases:

  • It's very unlikely that the default scope will be relevant for all cases, so you'll need to unscope sometimes
  • Changing it once it's established is very difficult, since only some sites declare their requirements
  • Everyone is going to be surprised by it every time they use the model

https://github.com/rubocop-hq/rubocop-rails/issues/76 lists these blog posts against it:

The existing list of uses is small, so I've added disable-comments for them in this MR. I know we had a large effort to remove default scopes from Project and other large models in the past. I think it's worth preventing any more from coming in now.

Check-list

  • Make sure this MR enables a static analysis check rule for new usage but ignores current offenses
  • Mention this proposal in the relevant Slack channels (e.g. #development, #backend, #frontend)
  • The MR doesn't have significant objections, and is getting a majority of 👍 vs 👎 (remember that we don't need to reach a consensus)
  • Create a follow-up issue to fix the current offenses as a separate iteration: #220817
  • Follow the review process as usual
  • Once approved and merged by a maintainer, mention it again:
    • In the relevant Slack channels (e.g. #development, #backend, #frontend)
    • (Optional depending on the impact of the change) In the Engineering Week in Review

/cc @gitlab-org/maintainers/rails-backend

cc @manojmj @minac as authors of the two referenced MRs above.

Edited by Nick Thomas

Merge request reports