return blank cleanup func for plugin.InmemCreationFunc #117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guygrigsby
approved these changes
Feb 6, 2024
|
This looks good - wherever you saw the panic can you also add a nil check there? Not sure if it was in this package or elsewhere. |
|
@jefferai I ran into the panic in HCP Boundary code, not sure it also happens elsewhere. This PR is referenced above |
jefferai
approved these changes
Feb 7, 2024
jefferai
requested changes
Feb 7, 2024
There was a problem hiding this comment.
On line 254 we create a cleanup func that is returned in success scenarios; depending on various conditions we modify that cleanup func later. Rather than create a func inline here, can you move creation of that cleanup above this (but returning nil) and simply reassign it on line 254?
That just keeps the return pattern consistent throughout.
|
@jefferai done! thank you! 😄 |
jefferai
approved these changes
Feb 13, 2024
|
Tagged v2.0.7. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Return a blank cleanup func instead of
nilto prevent accidental panic fromdefer cleanup()