[Merge] lp:~jamesodhunt/upstart/file-bridge-MP into lp:upstart

Dmitrijs Ledkovs launchpad at surgut.co.uk
Thu Mar 14 17:43:23 UTC 2013


On 11 March 2013 20:08, James Hunt <james.hunt at canonical.com> wrote:
> James Hunt has proposed merging lp:~jamesodhunt/upstart/file-bridge-MP into lp:upstart.
>
> === added file 'extra/man/file-event.7'
> --- extra/man/file-event.7      1970-01-01 00:00:00 +0000
> +++ extra/man/file-event.7      2013-03-11 20:07:26 +0000
> +.IP \(bu
> +If you wish to match on
> +.BR FMATCH ", "
> +ensure that
> +.B FPATH
> +does not contain multiple contiguous runs of slashes since otherwise
> +your job will find it difficult to perform such a match.
> +.\"


I didn't know the word "contiguous". I would have used "consecutive"
or "continuous".
Same for the comments in the code, where "contiguous" is also used.

> +/**
> + * expand_path:
> + *
> + * @parent: parent,
> + * @path: path.
> + *
> + * Expand @path by replacing a leading '~/', './' or no path prefix by
> + * the users home directory.
> + *
> + * Limitations: Does not expand '~user'.
> + *
> + * Returns: Newly-allocated fully-expanded path, or NULL on error.
> + **/

> +/**
> + * path_valid:
> + *
> + * @path: path.
> + *
> + * Perform basic tests to determine if @path is valid for
> + * the purposes of this bridge.
> + *
> + * Returns: TRUE if @path is acceptable, else FALSE.
> + **/

So in both of the above, no variable substitutions? Or the upstart
side of things would have replaced variables already for us (didn't
check thoughtfully)?
I am thinking about stuff like watching for file or directory that has
$(INSTANCE_NAME) in it.
I guess there should always be a "one up" location to watch for.

Overall the approach taken is sound and easy to follow and the code is
beautiful as usually. Building and testing here locally for now.

Have you considered using dbusmock for testing this bridge? (i guess
same applies to other bridges as well)

Regards,

Dmitrijs.

-- 
https://code.launchpad.net/~jamesodhunt/upstart/file-bridge-MP/+merge/152767
Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/file-bridge-MP into lp:upstart.



More information about the upstart-devel mailing list