hi,
On Thu, Sep 05, 2019 at 09:54:22AM +0200, Fabian Grünbichler wrote:
> On September 4, 2019 3:59 pm, Oguz Bektas wrote:
> > the description doesn't match the default behaviour, which is to replace
> > the current values with pending ones in the returned config, unless the
> > 'current' option is passed.
> >
> > Signed-off-by: Oguz Bektas <o.bek...@proxmox.com>
> > ---
> >
> > i tried to come up with a reasonable description, but sending it as RFC
> > just in case someone else comes up with something better
> >
> > PVE/API2/Qemu.pm | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> > index b30931d..36487ee 100644
> > --- a/PVE/API2/Qemu.pm
> > +++ b/PVE/API2/Qemu.pm
> > @@ -827,7 +827,8 @@ __PACKAGE__->register_method({
> > path => '{vmid}/config',
> > method => 'GET',
> > proxyto => 'node',
> > - description => "Get current virtual machine configuration. This does
> > not include pending configuration changes (see 'pending' API).",
> > + description => "Get virtual machine configuration. If the 'current'
> > option is" .
> > + " not passed, pending changes will replace the current
> > ones.",
>
> besides style, this is also incorrect since you can pass '0' for
> 'current' ;)
passing 0 to 'current' means the default behaviour, i don't think anyone
will do that :P
but we could rephrase like:
...
If the current option is not set, pending changes will replace the
current ones.
>
> Get virtual machine configuration. If 'current' is set, any pending
> configuration changes will not be included in the returned value.
> Otherwise, the returned configuration values will represent the state
> with all pending changes applied.
>
> A bit verbose, but matching what it does. Alternatively, just
a bit? :O
>
> Get virtual machine configuration. See 'pending' API for current and
> pending configuration retrieval.
i find this one insufficient, since it doesn't explain the fact that the
current options will be replaced with the pending ones.
>
> and leave the rest to the parameter description?
>
> current -> "Get current values only (ignoring pending changes)."
>
> > permissions => {
> > check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
> > },
> > --
> > 2.20.1
> >
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> >
> >
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
