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

fix: add vm context field to primary_ip update func #435

Merged
merged 1 commit into from
Jul 24, 2023

Conversation

tagur87
Copy link
Contributor

@tagur87 tagur87 commented Jul 21, 2023

When the local context was added to the vm resource, the fields were not added to the primary ip update function, which caused the local context field to be unset when the resource_netbox_primary_ip resource was used.

@tagur87
Copy link
Contributor Author

tagur87 commented Jul 21, 2023

@fbreckle - small fix for local context bug.

@tagur87 tagur87 force-pushed the primary-ip-context branch from 682c2a8 to e8c98b1 Compare July 21, 2023 21:36
@fbreckle
Copy link
Collaborator

fbreckle commented Jul 24, 2023

Please add a test that fails without the fix. This way we can confirm that the change is actually needed and the chance for future regressions is reduced.

Edit: In this case, by "add", I mean "edit an existing test to cover the changed attribute"

@tagur87 tagur87 force-pushed the primary-ip-context branch from e8c98b1 to 2795f44 Compare July 24, 2023 15:55
@tagur87
Copy link
Contributor Author

tagur87 commented Jul 24, 2023

Please add a test that fails without the fix. This way we can confirm that the change is actually needed and the chance for future regressions is reduced.

Edit: In this case, by "add", I mean "edit an existing test to cover the changed attribute"

@fbreckle - sorry about missing that. I have added a test now that should cover this usage in the primary_ip resource.

@tagur87 tagur87 force-pushed the primary-ip-context branch from 2795f44 to 4a53a5b Compare July 24, 2023 15:57
When the local context was added to the vm resource, the fields were
not added to the primary ip update function, which caused the local
context field to be unset when the `resource_netbox_primary_ip` resource
was used.

Add extra test to ensure converage and prevent regression in the
future.
@tagur87 tagur87 force-pushed the primary-ip-context branch from 4a53a5b to 6cd5462 Compare July 24, 2023 16:20
@fbreckle fbreckle merged commit 83d9d7b into e-breuninger:master Jul 24, 2023
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