public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Philipp Hufnagl <p.hufnagl@proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>,
	Wolfgang Bumiller <w.bumiller@proxmox.com>
Subject: Re: [pve-devel] applied: [PATCH manager v4 0/2] fix #474: allow transfer from container/vms
Date: Thu, 31 Aug 2023 15:47:00 +0200	[thread overview]
Message-ID: <ee5fc461-1743-4fb3-99d5-1328a7016e2e@proxmox.com> (raw)
In-Reply-To: <1ab8d642-cf29-4635-87db-45acb0051d1d@proxmox.com>

Am 30/08/2023 um 14:53 schrieb Philipp Hufnagl:
> On 8/24/23 16:46, Thomas Lamprecht wrote:
>> And I agree with Dominik, UX isn't ideal, a warning that one or more VMID will
>> be moved out of there old Pool, if any, would be sufficient. Not sure if it'd be
>> better if that's a per-row hint, shown if the row is ticked (e.g., instead of the
>> Pool column) or a edit-window wide warning hint that gets made visible if any of
>> the selected VMIDs is in a Pool already.
>>
>> FWIW, and not directly related (i.e., can be it's own series), you could also fix
>> the s/Virtual Machine/Virtual Guest/ wording to avoid the confusion that one also
>> adds Container over this interface.
> 
> Sorry for the issue. It has been my first Patch on this scale.

Yeah, here to "blame" (exaggeration) is also Wolfgang applying the UI
side IMO a bit prematurely, without consulting Dominik or me for UX.

But it's still all the more important to be reactive to feedback
especially when starting out, otherwise reviewers might stop giving it
if they feel it's not heard anyway.

> As for the feature design itself: The UI could be improved by only showing vms assinged to a pool when the transfer/migrade check box is checked. This way it should be clear if it is a migration without the use of a popup.
> 
> Would that work? Feedback is most welcome :)


IMO that's a bit convoluted and still needs an extra step, as Dominik
and I both, rather avoid the checkbox completely.




  reply	other threads:[~2023-08-31 13:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-10 10:09 [pve-devel] " Philipp Hufnagl
2023-08-10 10:09 ` [pve-devel] [PATCH manager v4 1/2] fix #474: api: " Philipp Hufnagl
2023-08-10 10:09 ` [pve-devel] [PATCH manager v4 2/2] fix #474: ui: " Philipp Hufnagl
2023-08-14 10:36 ` [pve-devel] applied: [PATCH manager v4 0/2] fix #474: " Wolfgang Bumiller
2023-08-14 10:42   ` Dominik Csapak
2023-08-24 14:46     ` Thomas Lamprecht
2023-08-30 12:53       ` Philipp Hufnagl
2023-08-31 13:47         ` Thomas Lamprecht [this message]
2023-08-30 12:43     ` Philipp Hufnagl
2023-08-30 14:29       ` Dominik Csapak
2023-08-31  8:08         ` Philipp Hufnagl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ee5fc461-1743-4fb3-99d5-1328a7016e2e@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=p.hufnagl@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=w.bumiller@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal