Re: q: boolean variable handling policy

On Fri, Mar 28, 2008 at 07:15:25PM +0200, Michael Shigorin wrote:
> Basically boolean_is_true() is complemented by boolean_is_false()
> for cases when default behaviour might differ depending on
> "conscious yes/no", and then is_yes()/is_no() shortcuts for
> these.  The reason is explained below.

i definitely see adding two additional possible values for
booleans(on/off/0/1), one of which (0/1) is particularly confusing in
shell scripts, a variation of of a boolean_is_false that is *nearly*
equivalent to a negated boolean_is_true, and two additional aliases to
those functions as undesireable when i'm spending time merging patches
to support ALTLinux.

even if it's a good idea in the long-term, in this phase of things, it
makes things more difficult.

> Vagrant told he's uncomfortable with this revision:
...snip...
> <vagrantc> gvy: well, until we have that discussion, i reject the
>            changes.
> <gvy> but frankly, i'm surprised to see such strong opposition to
>       such simple thing
> <vagrantc> i've got to go now.

you didn't include the part of the conversation where i said i didn't
have much time right now(or something to that effect), but seem to find
it important to include my last comment... as if it were a response to
your last comment. i am hoping that was not intentional.

> So while this wasn't sorted out online, I'm taking discussion
> "offline" so that it doesn't demand immediate replies (but still 
> does need ones).
> 
> Got three of them: 
> 
> 1) whether there is any particular policy on boolean value
>    handling;

at some point in the past, i recall that we agreed on true/false, y/n
and yes/no, and that's what we implemented(although i did notice ldm was
a little more loosely defined). some of this was based on what was
historically used for booleans in LTSP, and some just on changes that
happened when ltsp5 came along. 

how many booleans do we really need to support? what does it really gain
us?

though seemingly small, it does require some additional effort to
support more potential boolean values, and i do not see how it actually
improves the situation to add more.

> 2) whether there's more general one (README-DEVELOPMENT-POLICY
>    hints there's none fixated)

we've come to all sorts of informal discussions that haven't really been
documented, which over time eventually means we basically don't have any
policy. :)

> 3) and how generally the collaboration is going on here.
 
> The latter might require some more bytes...
 
generally, what's been going on is people throw out an idea in irc
(sometimes on the mailing list) and discuss it. we attempt to come to
agreement on things, or at least grudging acceptance. failing that, it
seems appropriate for the status quo to remain in place. this allows for
ideas to percolate on the back burner, and frequently someone comes up
with something better than the original idea, even if it takes longer.
 
> I'm quite easy with sed -ir on is_yes/is_no but I'd really want
> to understand the rationale after the unsung pieces of
> server/doc/CodingStyle.

you're adding 6 or more additional lines of code, and the only thing
you're getting is a shorter name for the function, at the expense of
having multiple ways of doing the exact same thing.

additionally, my intuitive feel is that is_yes and is_no sounds a little
confusing. and while code is not spoken language, it almost feels to me
like the grammar is somehow... not quite right. this is probably very
subjective, though.
 
> Knowing that there's always a taste difference and technical
> difference, we've tried to avoid hitting the former pretty hard
> during preparation of that particular branch.
 
> It would be fine to understand that our code is not aesthetic, 
> or breaks things, or is not safe.  But with a reason.

i feel like i gave reasons in irc, although not as well explained as i
would have liked, maybe with a hint of impatience due to outside
factors. sorry about that.
 
> It is somewhat hard for me to argue on whether !boolean_is_true()
> is *NOT* equal to boolean_is_false(), because it isn't in my
> shell scripting practice (it's not that long though, since 1998).

well, it is not exactly equal, but it is pretty similar. the default
assumption with the code is that if a value is un-set, it is false. for
booleans that default to true, it needs to be defined somewhere like
ltsp_config.

> There ain't really booleans in POSIX shell.  So we use strings.
> These can be empty (even unset).  This does make a difference.
> This is where I consider there's a technical reason to reject 
> a taste, not a code.

if there was a strong desire and need for boolean_is_false that would
actually simplify the code, i think it's worth considering. although i'd
probably still do something like:

 boolean_is_false(){
    if [ -z "$1" ] || ! boolean_is_true "$1" ; then
        return 0
    fi
    return 1
 }

rather than spelling out all the possible values for false. but i think
best to leave unimplemented until it is actually needed.

> Further, it's hard to talk aesthetics when bits and pieces like
> this one are in place *as the last command in a script*:
> 
> [ -e /selinux/enforce ] && /sbin/restorecon -R $TFTPDIR &> /dev/null
...snip...

i'm not going to defend that code... :)

> I'm thankful to Vagrant for all the patience and help he's done
> to me, but now I frankly don't understand where the "taste" and
> "technical reason" border is here.

well, i was expecting to merge patches needed to get ALTLinux into
upstream, and mixed in with all the ALTLinux stuff was unrelated
changes. i was pressed for time, and said as much, and probably
shouldn't have put any time into it. instead, i decided to try and give
what feedback i could with a quick glance at it.

i understand that folks are new to bzr, and that bzr has some unpleasant
limitations with regard to cherry-picking, but in the end it's harder to
merge *just* the ALTLinux stuff because of intermingling unecessary
changes.

> I'm all for making LTSP scripts *beautiful*, currently not being
> even "good" on that scale.  

i don't really see this as a constructive comment. you're calling the
code ugly, without really defining what qualities you find beautiful or
good. the examples below don't really make it any clearer.

> (actually, LTSP4.2's startx made me crawl the wall and not look into
> it for three more months until could do it again :-(  no joke, one can
> ask our CTO)

and i think the recent changes to the ltsp5 xdmcp (formerly startx)
screen script are a *huge* improvement over that.
 
> For an example of decent shell scripts, I propose for your
> reading pleasure this excerpt from libshell by Alexey Gladkov
> (ALT Linux):
> 
> http://tinyurl.com/3bc3cm
> 
> and this one by Solar Designer (Openwall Project) and
> Dmitry Levin (ALT Linux, Openwall Project):
> 
> http://tinyurl.com/2z7h8b
 
> I think it might be a honor to see LTSP scripts at that level 
> of clarity, modularity, reliability and security as Owl ones.

hm. i really don't see any sort of universal clarity or understand what
you mean by reliability there. it really just looks like stylistic
differences to me, and i'm not particularly fond of the style. too much
squiggle :)

live well,
  vagrant

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_____________________________________________________________________
Ltsp-developer mailing list.   To un-subscribe, or change prefs, goto:
      https://lists.sourceforge.net/lists/listinfo/ltsp-developer
For additional LTSP help,   try #ltsp channel on irc.freenode.net