search

oetiker / znapzend

zfs backup with remote capabilities and mbuffer integration.
www.znapzend.org
GNU General Public License v3.0
603 stars 136 forks source link

Fix inheritLevels semantics from #507 #516

Closed jimklimov closed 3 years ago

jimklimov commented 3 years ago

...and also this message from perl -w runs:

Variable "%inheritLevels" will not stay shared at .../znapzend/t/../lib/ZnapZend/ZFS.pm line 475.
Variable "%inheritLevels" will not stay shared at .../znapzend/t/../lib/ZnapZend/ZFS.pm line 1228.
jimklimov commented 3 years ago

Key notes from discussions in #507 :

@jimklimov 13 days ago Author Contributor I originally thought to make it a bit mask, then settled for a number of magic values with specific meanings (commented above) that are a bit faster to compare than more meaningful strings or further variables - and we pass this codepath a lot when we do :\

@oetiker 12 days ago Owner oh I did not realize that ... please don't :) use a hash with speaking keys for the features you want enabled ...

@jimklimov I think I get the idea but did not find an example to rip off, do you have some handy please?

@oetiker 


my $inherit = {
all => 1,
greens => 0,
reds => 1
};

if ($inherit->{all} or $inherit->{greens}){ .... }



> @jimklimov
> Ah, just so? I thought there was something computationally cheaper, like C enums. Thanks, will try.
> UPDATE: Oh, these are not strings, so might be cheap...
> I suppose my ultimate fix is not what you wanted (ended up like an enum), but cursorily seems to work and steps away from arcane numbers to human names for them, but still uses the number underneath. Might revise later to have it as a flag map per your example, but I think it would be a refactor irrelevant to functionality at the moment :)

> @oetiker 8 days ago Owner
> as you mentioned :) sort of semi satisfying this way ...
> Sure if you would like to follow up with another one to fix this properly ... I am fine with merging this one now.
> The reason I am trying to catch these things is that they are non-ideomatic and tend to make my life harder when I have to figure out some odd behavior some time down the road. As I tend to be the one stuck with these projects in the long run.
jimklimov commented 3 years ago

Should be fixed after merging #519