Opened 9 years ago
Closed 7 years ago
#26906 closed Cleanup/optimization (fixed)
Factor out an AlterFooTogether operation from AlterUnique/IndexTogether
| Reported by: | Akshesh Doshi | Owned by: | Akshesh Doshi |
|---|---|---|---|
| Component: | Migrations | Version: | dev |
| 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 (4)
comment:1 by , 9 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 9 years ago
| Has patch: | set |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
comment:3 by , 9 years ago
| Has patch: | unset |
|---|---|
| Triage Stage: | Accepted → Someday/Maybe |
From the pull request, "we should probably redesign the Alter[Index|Unique]Together] classes before tackling this PR here."
Okay, let's see what it looks like.