Re: [pve-devel] [PATCH manager 2/2] GUI: efi disk: cleanup - var to const

[Matthias Heiserer <m.heiserer@proxmox.com>]

On 18.01.2023 15:03, Thomas Lamprecht wrote:
> Am 17/01/2023 um 13:17 schrieb Matthias Heiserer:
>> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
>> ---
>>   www/manager6/qemu/HDEfi.js | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/www/manager6/qemu/HDEfi.js b/www/manager6/qemu/HDEfi.js
>> index ef3092a6..f6b7fc26 100644
>> --- a/www/manager6/qemu/HDEfi.js
>> +++ b/www/manager6/qemu/HDEfi.js
>> @@ -9,13 +9,13 @@ Ext.define('PVE.qemu.EFIDiskInputPanel', {
>>       vmconfig: {}, // used to select usused disks
>>   
>>       onGetValues: function(values) {
>> -	var me = this;
>> +	const me = this;
>>   
>>   	if (me.disabled) {
>>   	    return {};
>>   	}
>>   
>> -	var confid = 'efidisk0';
>> +	const confid = 'efidisk0';
>>   
>>   	if (values.hdimage) {
>>   	    me.drive.file = values.hdimage;
>> @@ -37,7 +37,7 @@ Ext.define('PVE.qemu.EFIDiskInputPanel', {
>>       },
>>   
>>       setNodename: function(nodename) {
>> -	var me = this;
>> +	const me = this;
>>   	me.down('#hdstorage').setNodename(nodename);
>>   	me.down('#hdimage').setStorage(undefined, nodename);
>>       },
>> @@ -50,7 +50,7 @@ Ext.define('PVE.qemu.EFIDiskInputPanel', {
>>       },
>>   
>>       initComponent: function() {
>> -	var me = this;
>> +	const me = this;
>>   
>>   	me.drive = {};
>>   
>> @@ -96,9 +96,9 @@ Ext.define('PVE.qemu.EFIDiskEdit', {
>>   
>>       width: 450,
>>       initComponent: function() {
>> -	var me = this;
>> +	const me = this;
>>   
>> -	var nodename = me.pveSelNode.data.node;
>> +	const nodename = me.pveSelNode.data.node;
>>   	if (!nodename) {
>>   	    throw "no node name specified";
>>   	}
> 
> I'm not that huge fan of applying JavaScript's const broadly to non-scalar values,
> as it's only a shallow constant and immutable; so no effect on array/object member.

I quite like const, especially with `const me = this`, because it tells 
me that I don't have to worry about the variable being reassigned.
But as long as there's a guideline on whether to use it, I'm happy :)
> 
> I mean we can start to use it more for scalar variables, as eslint catches overriding
> the const variable itself directly - but it probably makes sense to add some basic
> rules w.r.t. when/how it should be used over `let` in our JS style guide, something
> along the line of:
> 
> "Using `let` is fine in most context, and while `const` can have some advantages and
> possibly even avoid some specific bugs, it should only be applied to scalar values but
> not objects or arrays."
> 
> What do you think (apart from grammar/typos ;-))?

Looks good