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

Clean-out service_id from manifest when deleting a service. #268

Merged
merged 4 commits into from
May 11, 2021

Conversation

Integralist
Copy link
Collaborator

@Integralist Integralist commented May 7, 2021

Problem: When you delete a service, the service_id in the manifest persists. Which (depending on what subcommands you execute next) could cause an unexpected error.
Solution: When deleting a service, be sure to clean-out the Service ID value from the manifest.

@phamann
Copy link
Member

phamann commented May 10, 2021

This seems to be based off of #266 and thus makes it hard to review. Is this intentional?

@Integralist
Copy link
Collaborator Author

@phamann This was intentional. I purposely didn't ping you for a PR review as I was waiting for the other PR to be merged (thus cleaning up the extra changes in this PR). To be fair I should probably have made this a 'Draft PR' instead.

@Integralist Integralist force-pushed the integralist/clean-sid-manifest branch from 72d4606 to 13c1b0a Compare May 10, 2021 16:33
@Integralist Integralist added the bug Something isn't working label May 10, 2021
@Integralist Integralist marked this pull request as draft May 10, 2021 16:48
@Integralist Integralist force-pushed the integralist/clean-sid-manifest branch from 13c1b0a to e719ebb Compare May 11, 2021 12:48
@Integralist Integralist marked this pull request as ready for review May 11, 2021 12:49
@Integralist Integralist requested a review from kailan May 11, 2021 12:49
pkg/service/delete.go Outdated Show resolved Hide resolved
pkg/service/delete.go Outdated Show resolved Hide resolved
@Integralist Integralist force-pushed the integralist/clean-sid-manifest branch from e719ebb to 4870555 Compare May 11, 2021 12:58
@Integralist Integralist force-pushed the integralist/clean-sid-manifest branch from 4870555 to a311bc9 Compare May 11, 2021 13:05
Copy link
Member

@kailan kailan left a comment

Choose a reason for hiding this comment

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

Fantastic!

@Integralist Integralist merged commit 9149a66 into main May 11, 2021
@Integralist Integralist deleted the integralist/clean-sid-manifest branch May 11, 2021 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants