[bug#55893] [PATCH] gnu: python-xyz: Add python-pysdl2.

[Maxime Devos]

Jean Pierre De Jesus DIAZ via Guix-patches via schreef op vr 10-06-2022
om 18:09 [+0000]:
> +    (native-inputs
> +      (list sdl2 sdl2-image sdl2-gfx sdl2-mixer sdl2-ttf))

These need to be in 'inputs', not native-inputs -- their shared
libraries will actually be executed when python-pysdl2 executed, which
can only work if they are compiled for the same architecture as python-
pysdl2 is compiled for (that's what 'inputs' means; for 'native-
inputs', it would be compiled for the architecture on which python-
pysdl2 is compiled, not the architecture it is compiled for). 

> +    (synopsis "Python ctypes wrapper around SDL2")

ctypes sounds like an implementation detail not relevant to users of
python-pysdl2, maybe: ‘Python bindings around SDL2’?

> +                ; Disable pysdl2-dll. Not needed.

Nitpick: the convention is two ;;, not a single ;.

> +                 (string-append "DLL(\"SDL2\", [\"SDL2\", \"SDL2
2.0\","
> +                                "\"SDL2-2.0.0\"], "
> +                                "\""

Thee strings above can be combined.

> +                                (dirname
> +                                  (search-input-file inputs
> +                                                     "/lib/libSDL2.so"))

Indentations seems a bit wonky -- if this is to not make the line too long,
maybe try putting a line break between the 'string-append' and the "DLL(..."?

> +                                "\""
> +                                ")")))

These strings too.

> +    (arguments
> +      `(#:tests? #f ; Requires /dev/dri, OpenGL module, etc.
> +        #:phases
> +        (modify-phases %standard-phases

Recommended style (considered more readable):

  (list #:tests? #f ; etcetera
        #:phases
        #~(modify-phases [etcetera]))

(Many other packages don't do it like that yet, it has only
be discovered recently -- I would point you at IRC logs but
I'm currently offline.)

Also, don't put the package definition simply at the end, that
leads to merge conflicts.  Instead, try keep packages
alphabetical ... which is difficult here, because it has
historically neglected alphebetical ordening, but maybe right
after python-py would be a good fit?

Otherwise, the package definition LGTM from a distance, though
I only looked at the definition, I didn't check the source code
(for simplifying the substitute*-ions or checking for malware)
or build it.

Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part