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.