Code review comment for lp:~johnf-inodes/bzr/serve-init

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Ferlito wrote:
> On Thu, Oct 08, 2009 at 05:59:29PM -0000, John A Meinel wrote:
>> Review: Needs Fixing
>> # Specify startup options for bzr --serve
>>
>> ^- shouldn't that be "for bzr serve"
>
> Nice catch. Pushed.
>
>> Also, shouldn't DAEMON_ARGS default to "serve --port..."
>>
>> The way you have it written, seems like it will try to run:
>>
>> "bzr --port=127.0.0.0:4155"
>>
>> Which wouldn't actually do anything.
>
> The init script does
>
> bzr serve $DAEMON_ARGS --directory $REPO
>
> so it should o the right thing.

Yeah, I missed it because of the line wrap and '--'
+ start-stop-daemon --start --quiet --make-pidfile --pidfile $PIDFILE \
+ --chuid $USER:$GROUP --background --exec $DAEMON --test > /dev/null \
+ || return 1
+ start-stop-daemon --start --quiet --make-pidfile --pidfile $PIDFILE \
+ --chuid $USER:$GROUP --background --exec $DAEMON -- \
+ serve $DAEMON_ARGS --directory $REPO \
+ || return 2

^- --exec $DAEMON -- serve ...

I see it now, but it certainly wasn't obvious at first.

>
>
>> So I think it needs a couple small tweaks, and it would probably be good to get this licensed as (c) Canonical if johnf doesn't object. (It makes managing the code base easier when we don't have to think again about the license. if it is a problem for johnf, then we can consider further.)
>
> I'll sort that out shortly.
>
> Cheers,
> John
>

Thanks.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrP/70ACgkQJdeBCYSNAAO3ZgCeN6eysLXon/LPQm9Qo3Kw+gVc
D84An33yvK/NVEt6+vE5imoiHa/NbWM2
=DwMq
-----END PGP SIGNATURE-----

« Back to merge proposal