Re: [PATCHES] Add sdl2-image, sdl2-mixer, and sdl2-ttf

[Thompson, David]

On Thu, Dec 17, 2015 at 5:18 PM, Ricardo Wurmus <address@hidden> wrote:
>
> Thompson, David <address@hidden> writes:
>
>> From f0109ade56e024bf4ac0b6e1ac9ce3d0592964d1 Mon Sep 17 00:00:00 2001
>> From: David Thompson <address@hidden>
>> Date: Tue, 15 Dec 2015 13:30:11 -0500
>> Subject: [PATCH 1/3] gnu: Add sdl2-image.
>
>> * gnu/packages/sdl.scm (propagated-inputs-with-sdl2): New procedure.
>>   (sdl2-image): New variable.
>
> Not sure, but I think the (new) rules are to not indent the parentheses
> on the second line.

Fixed.

>> +(define (propagated-inputs-with-sdl2 package)
>> +  (map (match-lambda
>> +         (("sdl" _)
>> +          `("sdl2" ,sdl2))
>> +         (other other))
>> +       (package-propagated-inputs package)))
>
> Is it really necessary to map over all inputs?  Aren’t you just
> replacing a single input with another?  Maybe a more generic procedure
> to replace a named input would be better?  (It probably doesn’t really
> matter much.)

It's not necessary to map once you've reached the element you need to
replace, but the inputs list is small so I didn't care to write the
more optimized version.

>> +(define-public sdl2-image
>> +  (package (inherit sdl-image)
>> +    (name "sdl2-image")
>> +    (version "2.0.0")
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri
>> +               (string-append 
>> "http://www.libsdl.org/projects/SDL_image/release/SDL2_image-";
>> +                              version ".tar.gz"))
>> +              (sha256
>> +               (base32
>> +                "0d3jlhkmr0j5a2dd5h6y29jfcsj7mkl16wghm6n3nqqp7g3ib65j"))))
>> +    (propagated-inputs
>> +     (propagated-inputs-with-sdl2 sdl-image))))
>> +
>
> Looks good.
>
>> From 9c3d05c07c941e8031c583abb8acfe91fed11d12 Mon Sep 17 00:00:00 2001
>> From: David Thompson <address@hidden>
>> Date: Tue, 15 Dec 2015 13:30:56 -0500
>> Subject: [PATCH 2/3] gnu: Add sdl2-mixer.
>
>> * gnu/packages/sdl.scm (sdl2-mixer): New variable.
>
> [...]
>
>> +(define-public sdl2-mixer
>> +  (package
>> +    (inherit sdl-mixer)
>
> This is inconsistent with the previous expression.  Should ‘(inherit
> ...)’ be on the same line as ‘(package’ or not?

Yes.  Fixed.

>> +    (name "sdl2-mixer")
>> +    (version "2.0.0")
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri
>> +               (string-append 
>> "http://www.libsdl.org/projects/SDL_mixer/release/SDL2_mixer-";
>> +                              version ".tar.gz"))
>> +              (sha256
>> +               (base32
>> +                "0nvjdxjchrajrn0jag877hdx9zb788hsd315zzg1lyck2wb0xkm8"))))
>> +    (propagated-inputs
>> +     (propagated-inputs-with-sdl2 sdl-mixer))))
>> +
>
> OK!
>
>> From b0473e355c8661d77e575580e08852efa0060f36 Mon Sep 17 00:00:00 2001
>> From: David Thompson <address@hidden>
>> Date: Tue, 15 Dec 2015 13:31:32 -0500
>> Subject: [PATCH 3/3] gnu: Add sdl2-ttf.
>
>> * gnu/packages/sdl.scm (sdl2-ttf): New variable.
>
> [...]
>
>> +(define-public sdl2-ttf
>> +  (package
>> +    (inherit sdl-ttf)
>
> Same as above.

Fixed.

>> +    (name "sdl2-ttf")
>> +    (version "2.0.12")
>> +    (source (origin
>> +             (method url-fetch)
>> +             (uri
>> +              (string-append 
>> "http://www.libsdl.org/projects/SDL_ttf/release/SDL2_ttf-";
>> +                             version ".tar.gz"))
>> +             (sha256
>> +              (base32
>> +               "0vkg6lyj278mdpd52map3rfi65fbq16w67ahmmfcl77a8da60a47"))))
>> +    (propagated-inputs
>> +     (propagated-inputs-with-sdl2 sdl-ttf))))
>> +
>
> Otherwise okay.
> Thank you!

Thanks for the feedback.  Fixed and pushed.

- Dave