[pve-devel] applied: [PATCH container v2 2/2] pct: add keep-env option

[Thomas Lamprecht]

Am 02/02/2024 um 17:53 schrieb Thomas Lamprecht:
> Am 29/01/2024 um 16:43 schrieb Folke Gleumes:
>> The keep-env option allows the user to define if the current environment
>> should be kept when running 'pct enter/exec'. pct will now always set
>> '--keep-env' or '--discard-env' when calling lxc-attach to anticipate
>> the upcoming change in default behavior.
> 
> seems fine in general, but I saw a two code style nits, of which one I'd
> really like to avoid, see comment in line.
> 
>>
>> Signed-off-by: Folke Gleumes <f.gleumes at proxmox.com>
>> ---
>> This wasn't present in v1
>>
>>  src/PVE/CLI/pct.pm | 22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>>

applied, thanks!

Two small things though:
- there where two instances with double whitespace, e.g. after the
  `my @lxc_attach_cmd = `and before `$vmid`, and that then copied over
  to the other command too. It really did not seemed to be due to
  some vertical alignment, so I ameded the commit and dropped them.

- in general it's preferred to send new patches, single or series, as new
  thread, i.e., without in-reply too. I know some open source projects
  sometimes are fine with sending those as in-reply-to, but IMO that
  hides them more. It's naturally fine to propose ad-hock diffs/code
  in a discussion in thread, but if there's a full patch with a new
  revision, then one is basically always better off when sending that
  as new thread.

Oh, and while at it, w.r.t perl array-refs you nowadays can write
`@{$foo->{bar}}` as `$foo->{bar}->@*`, and you can concat lists,
so the final "add base command + user params then exec" part could
look like:

exec(@lxc_attach_cmd, '--', $foo->{bar}->@*);

I kept that as is for now, it's IMO really not worse if done explicitly.