-
-
Notifications
You must be signed in to change notification settings - Fork 35
feat: Add AlterField Operation to Migration Generator
#368
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
@seqre any feedback for me? |
seqre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This operation has many edge cases, and it should be thoroughly tested. Can you please add more tests first?
Think about possible cases, such as: name change (this one can be skipped for now, it's tricky), type change, attributes change, etc. All those should be either handled or visibly err when a given case is unhandled or ambiguous.
It seems that cot-cli/src/migration_generator.rs does not have a test for alter model, would you be so kind and add that too?
|
Sure, Thank you! |
3dac2e1 to
76b9835
Compare
|
@seqre I added more tests can you check if they are what you are expecting and any feedbacks next steps. Thanks! |
1df5eb5 to
c97bc84
Compare
|
@m4tx the error doesn't seem to come from my PR, can you take a look? |
Add
AlterFieldOperation to Migration GeneratorOverview
This PR introduces support for the
AlterFieldoperation in the migration generator.Related Issues
#205
Reviewer Notes