On Fri, 6 Nov 2009, Rogério Brito wrote:
> Hi, Christian.
Rogério,
> I would prefer if you could contribute patches that did fewer, more
> focused changes, like, say, one patch only for removing the useless
> quoting, other patch for changing the character classes (on top of the
> previous one, etc).
Sure.
> I like your changes, BTW. I only have some minor comments.
>
> > +SYMLINK_MODEL_DIR=/var/run/usbmount
>
> I'm not really sure that we should use a variable to carry a certain
> value if we don't plan on making it a changeable setting.
Then don't. But repeating the same path over and over again is not really
optimal and it's also error prone. Better using a variable and
setting it's value once.
> > # Exit if both vendor and model name are empty.
> > -test -n "$UM_VENDOR" || test -n "$UM_MODEL" || exit 0
> > +[ "$UM_VENDOR" ] || [ "$UM_MODEL" ] || {
>
> I'm not really sure if I like the short-circuiting idiom instead of a
> plain if-else.
Fine with me. Still, it's slightly more efficient and compact.
> Can you comment on favor of it? The elimination of the "test" command in
> place of the brackets is definitely welcome, as we had already talked
> about.
No question about using if-then-else-fi when there's an else, but if
there's only an if, I prefer that construct. Reverse logic is IMO also,
generally more usefull. WRT the status ($?), you usually care less for
the condition status and more for the consequences. Like in:
[ "$var" ] && do_this
where $? may be the result of the condition, vs.:
[ -z "$var" ] || do_this
where $? is the result of 'do_this'.
> > - if in_list "$fstype" "$FILESYSTEMS"; then
> > + if in_list "$FSTYPE" "$FILESYSTEMS"; then
>
> I prefer to have shell variables in caps. Are your changes just
> stylistic ones or with some other motivation?
Whereas I prefer caps for global variables only.
> > + log debug "surprise: action '$1'"
> > + exit 1
>
> Just nitpicking here, but, perhaps, if we are going to exit with an
> error, perhaps that should not be logged as a debug only message, but as
> an error.
Ok.
> I like your changes.
Great.
> Thank you for your contribution. If you want, you can co-maintain
> usbmount with me, to bring it to a better shape.
Sure.
> Thanks, Rogério Brito.
Thank you.
--
Cristian
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
