Date: Sat, 03 Apr 2004 16:31:04 -0800 From: Doug Barton <DougB@FreeBSD.org> To: John Baldwin <jhb@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/etc/rc.d mixer Message-ID: <406F5748.2080603@FreeBSD.org> In-Reply-To: <200403291155.43206.jhb@FreeBSD.org> References: <200403270926.i2R9QMiP083177@repoman.freebsd.org> <200403291155.43206.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
John Baldwin wrote:
> On Saturday 27 March 2004 04:26 am, Doug Barton wrote:
>
>>dougb 2004/03/27 01:26:22 PST
>>
>> FreeBSD src repository
>>
>> Modified files:
>> etc/rc.d mixer
>> Log:
>> A few small cleanups:
>>
>> 1. Add the shutdown keyword so that the script is run at shutdown time,
>> and the mixer* files are saved.
>
>
> Huh, it seemed to work in my testing w/o this change. Maybe I botched
> something.
Actually I think it might depend on how the system gets shut down.
Regardless, since you definitely want this to run at shutdown time,
adding the shutdown key word is the right way to go.
>> 2. Twiddle whitespace.
>
>
> Style(9) mandates blank lines at the start of functions with no local
> variables, so that's why I did that.
Weird. Not that big a deal either way actually, but I'll try to keep
this in mind next time.
>> 3. Remove an unecessary function, and therefore collapse one variable.
>
>
> Only unnecessary if you don't value abstraction. :) Now if you want to ever
> change the names of the mixer saved files due to a conflict or some such, you
> have to change it in multiple places rather than just one.
You're right, and if there were more than just two spots, or the same
file name was used in more than one script, it would be worth a level of
abstraction. But since neither of those is true, (and since the file
name is very unlikely to change), it's not worth it. Also, FYI, the way
you did it is not the typical "Bourne shell'ish" way to abstract a file
name. It would be much more common (and infinitely more efficient) to
simply set a script variable with the file name, and then use that
throughout the script.
> I also did post
> this script for review on @arch before it was committed, btw.
Yes, and I appreciated that at the time, and gave it a cursory review.
At the time I thought, "this is kind of weird, but doesn't look like it
will break anything." :) It was only after it stopped working for me
that I started digging into it more deeply, and then I figured if I'm
here already, might as well clean up a couple more items. Note, nothing
you did is really wrong, and I appreciate your contribution, as I'm sure
the other rc.d folks do as well.
Doug
--
This .signature sanitized for your protection
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?406F5748.2080603>