Add govc snapshot size command, standalone SnapshotSize function #2269
Conversation
|
@dougm Setting this as a draft since I've not squashed commits and finished cleaning up the branch yet. What are your preferences regarding code comments? I assumed that the project style was to omit them unless necessary and have pruned the majority of them back. a54302c represents that state, except for a few lines I wanted to get your feedback on. 7dfb85b represents a version of the code with comments, but without the "note to self" verbosity that I used to begin with as I explored the code. Is this form useful? |
atc0005
force-pushed
the
i2243-attempt-to-understand-snapshot-size-calc-logic
branch
from
a54302c
to
7de988e
Compare
|
@dougm I've made a pass to incorporate suggested changes. I've added light doc comments to the functions themselves, but not to the function body. Are those welcome, or is it preferred to keep them specific to what godoc exposes? |
Thanks @atc0005 , looks great. In general, I think it's preferred to have godoc comments convey the API functionality/contract and function body comments to document the implementation details. Looks like your current PR aligns with that. But I'm not too picky either, since personally I tend to read the source code as the source of truth, regardless of any API doc :) |
- govc: Add size (`s`) flag to `snapshot.tree` subcommand - Add `object.SnapshotSize` and helper functions to support calculating snapshot size from other client code Credit to @dougm for providing successive code examples, explanation and guidance for understanding the required steps to implement this support. As noted on vmwareGH-2243, code provided by @dougm was (AFAIK) based heavily on existing C# code for PowerCLI implementation of the `Get-Snapshot` cmdlet. - credit: @dougm - refs vmware#2243
atc0005
changed the title
atc0005
force-pushed
the
i2243-attempt-to-understand-snapshot-size-calc-logic
branch
from
b04190d
to
d3d49a3
Compare
|
@dougm Squashed commits and rebased on current |
Thank you for all of the help with this! I learned a lot along the way. |
atc0005
deleted the
i2243-attempt-to-understand-snapshot-size-calc-logic
branch
Fix gosec G601 "Implicit memory aliasing in for loop" error by reassigning the iteration variable inside the loop. refs vmwareGH-2243 refs vmwareGH-2269
Fix gosec G601 "Implicit memory aliasing in for loop" error by reassigning the iteration variable inside the loop. refs vmwareGH-2243 refs vmwareGH-2269
Add support for snapshot size calculations
s) flag tosnapshot.treesubcommandobject.SnapshotSizeand helper functionsto support calculating snapshot size from other
client code
Credit to @dougm for providing successive code examples,
explanation and guidance for understanding the required
steps to implement this support.
As noted on GH-2243, code provided by @dougm was (AFAIK)
based heavily on existing C# code for PowerCLI
implementation of the
Get-Snapshotcmdlet.