Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #35508: Add an --ignore-deps flag to squashmigrations (WIP) #18276

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pygeek
Copy link

@pygeek pygeek commented Jun 15, 2024

Trac ticket number

ticket-XXXXX

Branch description

Provide a concise overview of the issue or rationale behind the proposed changes.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

@pygeek pygeek force-pushed the ignore-deps-squashmigrations branch from 8e92781 to 2a7cb57 Compare June 15, 2024 07:27
Copy link
Member

@shaib shaib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this is just initial, so no detailed review, I just raised points which I suspect will save you some time in further work

django/core/management/commands/squashmigrations.py Outdated Show resolved Hide resolved

operations.extend(filtered_operations)
continue

operations.extend(smigration.operations)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be put under else:

Copy link
Author

@pygeek pygeek Jun 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was working through the logic, at that point, prior to changing the existing code. I just committed a more complete solution. Perhaps, you could clarify something else for me, however.

  1. How are test files created by migration commands cleaned up? I only see references for rewinding the state, but not deletion of created migration files.
  2. Should I exclude multi-app ModelOperations altogether, or mutate them to exclude fields that reference other apps?
  3. What would you recommend doing in the case of a SwappableTuple? Is there a predictable way to check if it's referencing an external app? Let me know if the logic I currently have makes sense.

I'm still writing tests. Also, let me know if I missed anything.

PS: I may decide to check the dependencies for multi-app dependencies, before combing through all of the operations to find references to other apps.

@pygeek pygeek force-pushed the ignore-deps-squashmigrations branch 3 times, most recently from 12fbb49 to 9284e5c Compare June 16, 2024 23:18
@pygeek pygeek force-pushed the ignore-deps-squashmigrations branch from 9284e5c to a712f86 Compare June 16, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants