From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 687C298C11 for ; Mon, 13 Nov 2023 15:58:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4003114694 for ; Mon, 13 Nov 2023 15:58:23 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 13 Nov 2023 15:58:22 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 80A4C41DC0 for ; Mon, 13 Nov 2023 15:58:22 +0100 (CET) Message-ID: <0fc05816-2b70-43b7-8efd-4e84cd6e651f@proxmox.com> Date: Mon, 13 Nov 2023 15:58:21 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: de-AT, en-US To: Dominik Csapak , Proxmox VE development discussion References: <20231107101827.340100-1-l.wagner@proxmox.com> From: Lukas Wagner In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.013 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters 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: , X-List-Received-Date: Mon, 13 Nov 2023 14:58:53 -0000 Hi, thanks for your input. On 11/13/23 15:34, Dominik Csapak wrote: > a few high level ui things > (i did not look too deeply in the code, but i'll send > probably some comments there too) > Just as a warning, the tree code/data binding is definitely not as clean as it could be right now, a cleanup patch will follow. I just wanted to get these patches out ASAP ;) > that probably was already there, but i find the all/any + invert > combination > confusing (i had to think about it for a bit before getting a grasp on it) > > i would propose we can write the four options out what they mean > and internally convert them to all/any + invert, e.g. > > 'all rule match' > 'any rule match' > 'at least one rule does not match' (all + invert) > 'no rule matches' (any + invert) I was considering this as well and discussed both approaches with Aaron. He was slightly in favor of the one I implemented, so I went with that. But now that I think about it again I think I like this version better, I'll send a followup for this (since this is is purely cosmetic). > > (also is the change from and/or to all/any not a breaking change?, > did we expose this in the api yet ?) Well, the switch-over from filters to matchers is breaking as a whole, but it only ever hit pvetest, it is not a big problem. (config parsing was adapted so that it will clean out 'old' entries) The notification stuff was merged a bit too eagerly, this rework attempts to fix some of the issues with the first approach. > > second, we already have a very similar interface in the guest wizard for > the disks, and there we have the remove button inline, > i guess we should keep the style consistent Noted, i'll put it on my list for followups. > > third, do we really need the tree? we basically have the > four options from above, and then a list of the matches > > wouldn't it be enough to seperate them? The tree was a conscious decision, because at some point I want to support 'sub-matchers' - where a matcher can reference another matcher. that way users can compose arbitrary boolean formulas, e.g.: All of match ... match ... any of match ... match ... For best UX, the sub-matchers shall be configurable within a single dialog, and this is the reason why I went with a tree widget. > > e.g. have a dropdown with the four options + a list instead of a tree? > > also currently the match options are not really verified before > i can submit the form > True, will fix this in a followup. Thx again! -- - Lukas