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

Develop 2.7 - New Connection Types #3844

Closed
wants to merge 5 commits into from
Closed

Develop 2.7 - New Connection Types #3844

wants to merge 5 commits into from

Conversation

ryanmerolle
Copy link
Contributor

Fixes #3841 (PowerPort & PowerOutlet)
Fixes #3842
Fixes #3843

@hSaria
Copy link
Contributor

hSaria commented Jan 3, 2020

I don't think migration are supposed to be modified.

@ryanmerolle
Copy link
Contributor Author

ryanmerolle commented Jan 3, 2020

For wireless I just added 802.11ax wherever I saw 802.11ad, and that included the migrations. I can remove those updates.

@DanSheps
Copy link
Member

DanSheps commented Jan 5, 2020

You should not directly modify previous migrations.

Instead, you should make the changes to trigger a migration and run manage.py makemigrations.

@ryanmerolle
Copy link
Contributor Author

Noob contribution on my part. I can remove, I just figured it would be needed for any where I saw similar wireless choices. How should I proceed?

I guess I did not also fully understand the migrations.

@DanSheps
Copy link
Member

DanSheps commented Jan 6, 2020

@ryanmerolle You would revert the changes to the migration files then run the python command:

python manage.py makemigrations --name adescriptivename

This will generate a new migration file, if required, that has the changes.

@DanSheps
Copy link
Member

DanSheps commented Jan 6, 2020

If the migration is not quite right, you can delete the migration file and re-create it with the same command.

@jeremystretch
Copy link
Member

Addressing three different FRs requires three different pull requests. I realize this may seem excessive, but experience has proven that trying to batch changes leads to problems and confusion.

You should not directly modify previous migrations.

This is generally correct. However, choice fields are a bit odd given that the resulting migration has no real effect on the underlying database scheme: it merely tracks changes to the Django model field. I have in the past merged these changes into prior migrations, but doing so takes considerable forethought and attention to detail, and I wouldn't think it fair to expect this of anyone who is not intimately familiar with NetBox's migration history.

So as far as PRs are concerned, it's best to submit each with its own discrete migration. I'm happy to condense them as the opportunity arises prior to a release.

@ryanmerolle I'm going to close out this PR to avoid confusion; could you please submit the changes proposed here as separate PRs?

@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants