Discourage splitting DB migrations from backend changes

Why is this change being made?

After I previously started a discussion about changing/removing this whole section about splitting MRs !58851 (merged) we learnt that there are quite a lot of concerns with splitting frontend changes from backend changes.

But, based on feedback from the database team, as well as most of the comments I could see from backend maintainers it appeared the general preference was to ensure database changes are made alongside the backend code that uses them.

Specifically, the only time I can think when people are splitting these 2 areas in the past, have been when someone is creating a new table/column and opt to create those tables/columns (database migrations) in a single MR then immediately follow up with another MR which uses the new table/column. In this case both MRs will require review from ~database and backend anyway to ensure that the modelling is sound from a database performance/scaling perspective as well as from a Rails code perspective and the reviewers will need to cross-reference the 2 separate MRs.

From the discussions with database and backend maintainers I could find the vast majority preferred a combined MR to provide a smoother review process.

Q&A

Will this noticeably impact our MR rate

My assumption is that this will have a low impact on our MR rate. As I've stated previously in !58851 (comment 417791993) I think lowering MR rate alone is not necessarily a great reason to want to avoid making changes like this. However, in this case, it will likely be low impact since I very infrequently see MRs that contain only database migrations. The main reason for this is that we change the database much less frequently than changing other parts of the application. Another reason why this is less common, I assume, is that few engineers are actually following this advice today about splitting database changes out.

It's also possible that, per the discussions in gitlab-org/database-team/team-tasks#93 (closed) this may reduce the mean time to merge as it saves the database and backend reviewers from having to cross-check different MRs or ask additional questions to the author to clarify things before approving since the context will all be in the single MR.

Why are we changing this now, what's changed?

There has been conversation at length in !58851 (merged) with varying opinions about which approach to slicing MRs is preferred and I don't want to cover all that here. But it's worth noting that these suggestions have been added in !47946 (merged) and there wasn't all that much discussion about the tradeoffs when it was introduced and after it was merged the knowledge wasn't shared very widely so I actually don't think that many people have been doing this up until now anyway. Having these preferences determined by the careful discussion from @gitlab-org/maintainers/rails-backend and @gitlab-org/database-team seems like the best way to make an informed decision so I hope they can raise any concerns they have here before we merge this.

Does this mean that you can't change anything in the database without changing the backend?

No. I believe the language here was really only referring to the case where a feature being implemented required additional database columns. I think there are cases where you will change only the database (eg. adding a new index) and in those cases we don't need to make up random backend changes to add alongside it. I'd like to understand if there is something other than a migration adding new unused tables/columns where we need to discuss a different preference here.