Re: [PATCH] Add IcedTea 7.

[Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>]

Ludovic Courtès writes:

>> * gnu/packages/java.scm (icedtea7): New variable.
>
> [...]
>
>> +         #:locale "C"
>
> Could you add a comment explaining why?

Done

>> +         ,@(substitute-keyword-arguments `(#:modules ((guix build gnu-build-system)
>> +                                                      (guix build utils)
>> +                                                      (srfi srfi-1)
>> +                                                      (srfi srfi-26))
>> +                                                     ,@(package-arguments icedtea6))
>
> The ,@ should be aligned with #:modules (unfortunately Emacs fails to do
> that by default.)

I noticed that the #:modules stuff was not actually required, so it
looks nicer now.

>
>> +             ((#:phases phases)
>> +              `(alist-replace
>
> This is typically a case where ‘modify-phases’ would be more readable,
> if you feel like changing it.

I rewrote this to use the 'modify-phase' syntax and pushed to master.
Thank you for the review!

~~ Ricardo