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

[Jean Pierre De Jesus DIAZ]

Applied the suggestions to the code and relocated the definition of the package 
on the `python-py` packages section.

Thanks for the review.

> ------- Original Message -------
> On Saturday, June 11th, 2022 at 12:13 AM, Maxime Devos maximedevos@telenet.be 
> wrote:
>
>
>
> > 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.

0001-gnu-python-xyz-Add-python-pysdl2.patch
Description: Text Data