Vsphere: list snapshot by description #1555
Merged
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
|
Thanks for submitting this pull request 🎉. The team will review it soon and get back to you. If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document. |
AKhoria
changed the title
ihcsim
reviewed
Jul 20, 2022
ihcsim
reviewed
Jul 20, 2022
AKhoria
force-pushed
the
ankhoria/vshpere-test-list-snapshots
branch
2 times, most recently
from
1c769aa
to
0e36f86
Compare
AKhoria
force-pushed
the
ankhoria/vshpere-test-list-snapshots
branch
from
0e36f86
to
babe899
Compare
|
Code change LGTM. @carlbraganza @bathina2 PTAL and see if it's mergeable. Thanks. |
AKhoria
force-pushed
the
ankhoria/vshpere-test-list-snapshots
branch
from
b83aae4
to
26b775e
Compare
pkg/blockstorage/vmware/vmware.go
Outdated
| return res, err | ||
| } | ||
|
|
||
| func (p *FcdProvider) getCreatedSnapshot(ctx context.Context, description string, volID string, notEarlierThan time.Time) interface{} { |
There was a problem hiding this comment.
Why a return type of interface{}? You are treating the returned value as a bool in the calling function so use that type!
There was a problem hiding this comment.
We are treating its as Types.ID, I've changed the code to returnung Types.ID explicitly
denisvmedia
reviewed
Aug 1, 2022
denisvmedia
reviewed
Aug 1, 2022
denisvmedia
reviewed
Aug 1, 2022
denisvmedia
reviewed
Aug 1, 2022
denisvmedia
reviewed
Aug 1, 2022
denisvmedia
reviewed
Aug 1, 2022
denisvmedia
reviewed
Aug 1, 2022
carlbraganza
approved these changes
Aug 16, 2022
r4rajat
pushed a commit
to r4rajat/kanister
that referenced
this pull request
Aug 28, 2022
* vmware ListSnapshot by description * vmware ListSnapshot by description * vmware check if failed snapshot creation created snapshot * vmware check if failed snapshot creation created snapshot * vmware check if failed snapshot creation created snapshot * vmware moving snapshot description out of loop * fix by review * fix by review * add comments * fix by review * fix by review * fix by review * fix by review Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.
Change Overview
Add alternative way to list snapshots in Fcd provider by using description field
Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan