[Libguestfs] [PATCH] builder: output translated notes

Pino Toscano ptoscano at redhat.com
Thu Jan 30 16:10:41 UTC 2014


On Thursday 30 January 2014 15:52:05 Richard W.M. Jones wrote:
> On Thu, Jan 30, 2014 at 02:44:12PM +0100, Pino Toscano wrote:
> > +  let l = ref [] in
> 
> 'xs' is a bit more natural for lists of things than 'l'.
> 
> > +  if Str.string_match regex loc 0 then (
> > +    let match_or_empty n =
> > +      try Str.matched_group n loc with
> > +      | Not_found -> "" in
> 
> My preference is to put 'in' on a separate line if you're defining a
> function (as here), and 'in' at the end of the line if you're defining
> a value.
> 
> So I would have written this:
> 
>   let match_or_empty n =
>     try Str.matched_group n loc with Not_found -> ""
>    in

Ah OK.

> > +        let notes = List.fold_left (
> > +          fun acc lang ->
> > +            match List.filter (
> > +              fun (langkey, _) ->
> > +                match langkey with
> > +                | "C" -> lang = ""
> > +                | langkey -> langkey = lang
> > +            ) notes with
> > +            | (_, noteskey) :: _ -> noteskey :: acc
> > +            | [] -> acc
> > +        ) [] langs in
> 
> I've noticed you like to put huge expressions between match .. with!

You are right, it started with less stuff, and then I didn't realize how 
big it became :) I just split as you hinted above, and also for the 
List.rev notes.

> Rest seems fine so ACK.

Fixed and pushed, thanks.

-- 
Pino Toscano




More information about the Libguestfs mailing list