Re: [pbs-devel] [PATCH proxmox-backup v4 1/3] fix #4315: jobs: modify GroupFilter so include/exclude is tracked

[Philipp Hufnagl <p.hufnagl@proxmox.com>]

On 12/15/23 10:44, Lukas Wagner wrote:
> 
> 
> On 12/4/23 11:04, Philipp Hufnagl wrote:
>> After some discussion I canged the include/exclude behavior to first
>> run
>> all include filter and after that all exclude filter (rather then
>> allowing to alternate inbetween). This is simply done by sorting the
>> list (include first) before executing it.
>>
>> Since a GroupFilter now also features an behavior, the Struct has been
>> renamed To GroupType (since simply type is a keyword). The new
>> GroupFilter now has a behaviour as a flag 'is_exclude'.
>>
>> I considered calling it 'is_include' but a reader later then might not
>> know what the opposite of 'include' is (do not include? 
>> deactivate?). I
>> also considered making a new enum 'behaviour' but since there are
>> only 2
>> values I considered it over engeneered.
>>
>> Matching a filter will now iterate with a forech loop in order to also
>> exclude matches.
>>
> 
> 
> Short summary of our off-list discussion:
> I think this would be a good opportunity to factor out the 'Applying a
> set of GroupFilters to a list of existing BackupGroups'-part into
> separate helpers, allowing us to write some tests for the filter
> logic. This would make it much easier to argue about the correctness
> of the four cases (includes, excludes, both, none).
> 

I think that is a great idea! I will extend this patch series with a
patch introducing testing!