From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 2985F1FF16E for ; Tue, 29 Oct 2024 13:22:49 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8E79536130; Tue, 29 Oct 2024 13:22:50 +0100 (CET) Date: Tue, 29 Oct 2024 13:22:46 +0100 From: Christoph Heiss To: Timothy Nicholson Message-ID: References: <20241029103956.33643-1-t.nicholson@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241029103956.33643-1-t.nicholson@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.030 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, vm.name] Subject: Re: [pve-devel] [PATCH manager v2] fix #5787: ui: add util function for guest confirmation dialog X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Cc: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" The patch title implies that it only adds a utility function, but the patch itself also includes usage for it. Something like "fix #5787: ui: include guest name in confirmation dialog" or similar would probably be a better fit. On Tue, Oct 29, 2024 at 11:39:56AM GMT, Timothy Nicholson wrote: > Signed-off-by: Timothy Nicholson > --- > changes since v1: > - modified function signature > - removed unnecessary questionmark Link to the previous version on lore.proxmox.com is always nice to have in the notes, FWIW. But no hard feelings, just pure convenience if one wants to look at previous versions :^) > > www/manager6/Utils.js | 4 ++++ > www/manager6/lxc/CmdMenu.js | 4 ++-- > www/manager6/lxc/Config.js | 8 +++++--- > www/manager6/qemu/CmdMenu.js | 4 ++-- > www/manager6/qemu/Config.js | 14 ++++++++------ > www/manager6/window/GuestStop.js | 2 +- > 6 files changed, 22 insertions(+), 14 deletions(-) > > diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js > index db86fa9a..493a635f 100644 > --- a/www/manager6/Utils.js > +++ b/www/manager6/Utils.js > @@ -1960,6 +1960,10 @@ Ext.define('PVE.Utils', { > } > return languageCookie || Proxmox.defaultLang || 'en'; > }, > + > + format_guest_confirmation: function(taskType, vmid, guestName) { Maybe name it `format_guest_task_confirmation`, to really tie it to the task functionality? > + return Proxmox.Utils.format_task_description(taskType, `${vmid} (${guestName})`); > + }, > }, > > singleton: true, > [..] > diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js > index d0e40fc4..a15ff815 100644 > --- a/www/manager6/lxc/Config.js > +++ b/www/manager6/lxc/Config.js > @@ -20,6 +20,8 @@ Ext.define('PVE.lxc.Config', { > throw "no VM ID specified"; > } > > + var vmname = vm.name; > + Any particular reason for this variable binding? Using `vm.name` below directly would work too, if I'm not overlooking something. It's not really shorter anyway. In any case, please use `const` (or `let` if needed) for new variable declaration instead of `var`. > var template = !!vm.template; > > var running = !!vm.uptime; > [..] > diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js > index f28ee67b..1a9d1ba9 100644 > --- a/www/manager6/qemu/Config.js > +++ b/www/manager6/qemu/Config.js > @@ -19,6 +19,8 @@ Ext.define('PVE.qemu.Config', { > throw "no VM ID specified"; > } > > + var vmname = vm.name; > + Same as above. > var template = !!vm.template; > > var running = !!vm.uptime; > [..] _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel