Opened 16 months ago

Last modified 16 months ago

#26906 assigned Cleanup/optimization

Factor out an AlterFooTogether operation from AlterUnique/IndexTogether

Reported by: Akshesh Doshi Owned by: Akshesh Doshi
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The reasons why I think this should be done are

1) When a bug is reported for any one of them (like #26180) the owner or reviewer has to check if the bug also exists (most of the time it does) for the other operation and apply the same fix for the other (see !6926), or else the fix might get delayed until another ticket is opened reporting the same issue for the other operation.

2) Recently I realized that a small part of AlterIndexTogether code is untested but tests exist for those lines for AlterUniqueTogether so having the same source of code for both the operations will result in better code coverage!

3) More than 90% of the code for both the operations is the same so in honor of the DRY rule (this is maybe kinda the reason for point 1 :P).

Change History (3)

comment:1 Changed 16 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

Okay, let's see what it looks like.

comment:2 Changed 16 months ago by Akshesh Doshi

Has patch: set
Owner: changed from nobody to Akshesh Doshi
Status: newassigned

comment:3 Changed 16 months ago by Tim Graham

Has patch: unset
Triage Stage: AcceptedSomeday/Maybe

From the pull request, "we should probably redesign the Alter[Index|Unique]Together] classes before tackling this PR here."

Note: See TracTickets for help on using tickets.
Back to Top