all lists on 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>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH cluster v8 3/4] datacenter.cfg: add option for tag-style
Date: Thu, 10 Nov 2022 10:54:09 +0100	[thread overview]
Message-ID: <e261b3c2-f6d0-8f23-a6ee-a08e5ae03f98@proxmox.com> (raw)
In-Reply-To: <20221018140226.598710-4-d.csapak@proxmox.com>

w.r.t. to this one and the next patch:

I'd slightly prefer either having just a single property for all, or split it into two:
style and rest. The latter may have a small benefit w.r.t. to stability - if a parser/edit
bug sneaks in that breaks styling the access stuff might be still working, but depends
really on what scenario one imagines.

so either:

tags => {
   ...
    format => {
        'color-map' => {},
        'shape' => {},
        'access' => {},
        'access-list' => {}
        'registered' => {} # admin tags 
    },
}

(I also altered the property names a bit to drop the slightly redundant "tag" from the
inner ones and use registered for admin tags (no hard feelings on the latter one))

or

'tag-style' => {
   ...
    format => {
        'color-map' => {},
        'shape' => {},
    },
},
'tag-access' => {
   ...
    format => {
        'user-allow' => {},
        'user-allow-list' => {}
        'registered' => {} # admin tags, if not separate.
    },
},

but I don't think the tree properties are bad at all, just seems a bit bloated, probably
just because we rarely expand datacenter.cfg. So, I won't block this if you want to stick
to your originally formats.




  reply	other threads:[~2022-11-10  9:54 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-18 14:02 [pve-devel] [PATCH cluster/qemu-server/container/wt/manager v8] add tags to ui Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH cluster v8 1/4] add CFS_IPC_GET_GUEST_CONFIG_PROPERTIES method Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH cluster v8 2/4] Cluster: add get_guest_config_properties Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH cluster v8 3/4] datacenter.cfg: add option for tag-style Dominik Csapak
2022-11-10  9:54   ` Thomas Lamprecht [this message]
2022-10-18 14:02 ` [pve-devel] [PATCH cluster v8 4/4] DataCenterConfig: add tag rights control to the datacenter config Dominik Csapak
2022-11-09 14:42   ` Aaron Lauterer
2022-11-10 10:09   ` Thomas Lamprecht
2022-10-18 14:02 ` [pve-devel] [PATCH qemu-server v8] api: update: improve tag privilege check Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH container v8] check_ct_modify_config_perm: " Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH widget-toolkit v8 1/2] add tag related helpers Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH widget-toolkit v8 2/2] Toolkit: add override for Ext.dd.DragDropManager Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 01/12] api: /cluster/resources: add tags to returned properties Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 02/12] api: add /ui-options api call Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 03/12] ui: call '/ui-options' and save the result in PVE.UIOptions Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 04/12] ui: parse and save tag color overrides from /ui-options Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 05/12] ui: tree/ResourceTree: collect tags on update Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 06/12] ui: add form/TagColorGrid Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 07/12] ui: dc/OptionView: add editors for tag settings Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 08/12] ui: add form/Tag Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 09/12] ui: add form/TagEdit.js Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 10/12] ui: {lxc, qemu}/Config: show Tags and make them editable Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 11/12] ui: tree/ResourceTree: show Tags in tree Dominik Csapak
2022-10-18 14:02 ` [pve-devel] [PATCH manager v8 12/12] ui: add tags to ResourceGrid and GlobalSearchField Dominik Csapak
2022-11-07 14:56 ` [pve-devel] [PATCH manager] ui: add missing tag classes Dominik Csapak
2022-11-09 13:11 ` [pve-devel] [PATCH cluster/qemu-server/container/wt/manager v8] add tags to ui Aaron Lauterer

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=e261b3c2-f6d0-8f23-a6ee-a08e5ae03f98@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal