-
Notifications
You must be signed in to change notification settings - Fork 910
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
correct SetControlFlags and ClearControlFlags APIs #2864
Conversation
3d3e640
to
f9dfa56
Compare
f9dfa56
to
a140aa8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. I don't know what's causing the linter action to fail though cc @embano1
https://github.com/vmware/govmomi/runs/6681587156?check_suite_focus=true
Yup, linter is complaining about the order of imports. Just run the proposed command from the failure execution accordingly to format the code. |
vslm/client_test.go
Outdated
"github.com/dougm/pretty" | ||
|
||
"github.com/vmware/govmomi" | ||
"github.com/vmware/govmomi/cns" | ||
cnstypes "github.com/vmware/govmomi/cns/types" | ||
"github.com/vmware/govmomi/vim25/soap" | ||
"github.com/vmware/govmomi/vim25/types" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix linter errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed linter issue
% golangci-lint run -v --fast=false
INFO [config_reader] Config search paths: [./ /Users/divyenp/go/src/github.com/divyenpatel/govmomi /Users/divyenp/go/src/github.com/divyenpatel /Users/divyenp/go/src/github.com /Users/divyenp/go/src /Users/divyenp/go /Users/divyenp /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 2 linters: [goimports govet]
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|imports|types_sizes|exports_file|files|name) took 1.103482096s
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 105.520716ms
INFO [linters context/goanalysis] analyzers took 0s with no stages
INFO [runner/skip dirs] Skipped 2 issues from dir cns/types by pattern cns/types
INFO [runner] Issues before processing: 19, after processing: 0
INFO [runner] Processors filtering stat (out/in): exclude: 17/17, filename_unadjuster: 19/19, skip_dirs: 17/19, autogenerated_exclude: 17/17, identifier_marker: 17/17, exclude-rules: 17/17, nolint: 0/17, cgo: 19/19, path_prettifier: 19/19, skip_files: 19/19
INFO [runner] processing took 2.894046ms with stages: nolint: 934.182µs, exclude-rules: 816.604µs, autogenerated_exclude: 665.116µs, identifier_marker: 303.128µs, path_prettifier: 109.216µs, skip_dirs: 53.233µs, cgo: 5.092µs, filename_unadjuster: 3.253µs, max_same_issues: 1.091µs, uniq_by_line: 630ns, skip_files: 364ns, source_code: 320ns, max_from_linter: 290ns, diff: 277ns, exclude: 230ns, path_prefixer: 222ns, severity-rules: 221ns, sort_results: 198ns, path_shortener: 194ns, max_per_file_from_linter: 185ns
INFO [runner] linters took 634.801611ms with stages: goanalysis_metalinter: 631.844527ms
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 20 samples, avg is 73.2MB, max is 74.1MB
INFO Execution took 1.85445765s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just linter errors need to be fixed
a140aa8
to
88e4df2
Compare
88e4df2
to
5929abf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Just noticed your commit message doesn't have a prefix, so it won't show up in the release notes section. Is this API related? If so, and if you want, please prefix the commit title with Happy to merge otherwise |
Thanks @embano1 I will follow this in the next commit. |
Description
This PR is correcting
SetControlFlags
andClearControlFlags
APIs to supply Volume ID.When we call these APIs without Volume ID, APIs fail with
ServerFaultCode: VolumeId is empty.
Please mark options that are relevant:
How Has This Been Tested?
Created VMDK and executed tests
This PR is required for kubernetes-sigs/vsphere-csi-driver#1762
cc: @chethanv28 @bandyopadhyays-vmware