Am 27.03.25 um 12:03 schrieb Wolfgang Bumiller: > On Tue, Mar 25, 2025 at 01:50:20PM +0100, Fiona Ebner wrote: >> Am 24.03.25 um 16:43 schrieb Wolfgang Bumiller: >>> Just a short high level nit today, will have to look more closely at >>> this and the series the next days: >>> >>> There's a `new()` which takes an $scfg + $storeid. >>> >>> But later there are some methods taking `$self` (which usually means the >>> thing returned from `new()`), which also get a `$storeid` as additional >>> parameter (but without any `$scfg`). IMO the `$storeid` should be >>> dropped there. >> >> Nice catch! Yeah, I think that was an oversight when I restructured in >> an earlier version. In fact, my example implementations of those >> functions even use $self->{storeid} already (or don't require the >> storeid at all). I'll remove those left-overs in v6. > > Two more small things: > > The `restore_get_{guest,firewall}_config` docs should probably > specifically mention that these are independent queries and called > without any `restore_*_init()` calls. > > Thinking about this more, maybe they should be renamed. > It might be nicer to have the `restore_` prefix used only for restore > processes, and rename these to `archive_get_*_config()`? > These are also used to view the config in the UI (or at least the > `get_geust_config` one is also called from > `Storage::extract_vzdump_config()`).
Agreed :) _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel