On 06/13/2017 09:01 AM, Michael Rasmussen wrote:
On Mon, 12 Jun 2017 14:18:59 +0200
Dominik Csapak <d.csa...@proxmox.com> wrote:
i just gave this patch a quick glance, here a few remarks:
a bit nitpicky, but your indentation needs fixing, the FreeNASEdit.js seems
mostly ok, but nearly every other file you touched has wrong indentation
The reason is that all the *.js files uses a mix of either tabs for
indentation or spaces, in some cases used intertwined so until somebody
normalizes all *.js files it seems the only safe way is to stick
exclusively with spaces as indentation and instruct your editor to
leave tabs untouched for not changes text.
not really, we try to also use the same indentation as with perl
see https://pve.proxmox.com/wiki/Perl_Style_Guide#Indentation
(one indentation level is 4 spaces, every 8 spaces are converted using tabs)
there are some files which do not use this style, but we try to fix
this as we editing those files (also patches are welcome :P )
+ xtype: 'textfield',
+ name: 'password',
+ emptyText: '',
+ inputType: 'password',
+ fieldLabel: gettext('Password'),
+ allowBlank: false
+ },
not really a gui problem, but it is probably a bad idea to save/load passwords
from/to a textfield, at least do not load and insert it in the field, but leave
it empty
How would you suggest fixing this? If you don't enter something then
the password will be change if the user presses save (I am not a
javascript guru;-)
in the onGetValues function you could do something like this:
if (values.password === '') {
delete values.password;
}
this only sends the password to the api when it is non empty
but a better way for the whole plugin would probably be a credentials
file with limited read access (so only root can read it)
+Ext.define('PVE.storage.FreeNASEdit', {
+ extend: 'PVE.window.Edit',
+
+ initComponent : function() {
+ var me = this;
+
+ me.isCreate = !me.storageId;
+
+ if (me.isCreate) {
+ me.url = '/api2/extjs/storage';
+ me.method = 'POST';
+ } else {
+ me.url = '/api2/extjs/storage/' + me.storageId;
+ me.method = 'PUT';
+ }
+
+ var ipanel = Ext.create('PVE.storage.FreeNASInputPanel', {
+ isCreate: me.isCreate,
+ storageId: me.storageId
+ });
+
+ Ext.apply(me, {
+ subject: 'FreeNAS Storage',
you add the storage type to format_storage_type, but here you do not use it, is
there a reason?
I don't get this. Please elaborate?
see for example the code in www/manager6/storage/ZFSPoolEdit.js
-----8<-----
Ext.apply(me, {
subject: PVE.Utils.format_storage_type('zfspool'),
isAdd: true,
----->8-----
This is also something in our code that could use some patches ;)
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
