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

feat: migration fix #227

Merged
merged 3 commits into from
Oct 7, 2024
Merged

feat: migration fix #227

merged 3 commits into from
Oct 7, 2024

Conversation

zacharis278
Copy link
Contributor

@zacharis278 zacharis278 commented Oct 7, 2024

Description:

During the Python 3.11 upgrade (#189) a few lines got dropped from our Makefile that allowed us to override the common constraints for django-simple-history. This caused us to automatically revert back to 3.0.0 when the upgrade bot ran which produces a slightly different makemigrations output. Migrations created by versions <3.1.1 will cause edx-platform checks to fail since those checks include running makemigrations using django-simple-history=3.4.0 and ensuring there are no changes.

Pre-Merge Checklist:

  • Updated the version number in edx_name_affirmation/__init__.py if these changes are to be released. See OEP-47: Semantic Versioning.
  • Described your changes in CHANGELOG.rst.
  • Confirmed Github reports all automated tests/checks are passing.
  • Approved by at least one additional reviewer.

Post-Merge:

  • Create a tag matching the new version number.

Copy link

github-actions bot commented Oct 7, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  edx_name_affirmation
  __init__.py
Project Total  

This report was generated by python-coverage-comment-action

Copy link
Member

@ilee2u ilee2u left a comment

Choose a reason for hiding this comment

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

LGTM, just have a clarifying question for my own sake below.

]

operations = [
migrations.AlterModelOptions(
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, how are you doing a migration w/o a change to the model being present in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The model actually did change just not in our code. It was the base class for that model, TimeStampedModel that is now different.

@zacharis278 zacharis278 merged commit 1d0ea6d into main Oct 7, 2024
6 checks passed
@zacharis278 zacharis278 deleted the zhancock/migration-fix branch October 7, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants