From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id E34461FF16E for ; Tue, 29 Oct 2024 13:53:52 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F0D51367E2; Tue, 29 Oct 2024 13:53:54 +0100 (CET) Mime-Version: 1.0 Date: Tue, 29 Oct 2024 13:53:22 +0100 Message-Id: From: "Shannon Sterz" To: "Proxmox VE development discussion" , "Timothy Nicholson" X-Mailer: aerc 0.17.0-69-g65571b67d7d3-dirty References: <20241029103956.33643-1-t.nicholson@proxmox.com> In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL -0.044 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On Tue Oct 29, 2024 at 1:22 PM CET, Christoph Heiss wrote: > 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? > just chiming in here as a reminder that our js guidelines want you to use camelCase for new variables/functions [1]. [1]: https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing > > + 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 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel