Subject: Re: [bug#62550] [PATCH v2] gnu: Add alienblaster.
From: Nicolas Goaziou <[email-masked]>
To: Yovan Naumovski via Guix-patches via <[email-masked]>
Cc: [email-masked], [email-masked], Yovan Naumovski <[email-masked]>
Date: Fri, 21 Apr 2023 10:59:27 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

Yovan Naumovski via Guix-patches via <[email-masked]> writes:

> * gnu/packages/games.scm (alienblaster): New variable.

Thank you. I applied your patch with the following changes.

> +       (uri (string-append "http://www.schwardtnet.de/alienblaster/archives/"
> +                           name "-" version ".tgz"))

I replaced name with alienblaster.

> +       (sha256
> +        (base32
> +         "104rfsfsv446n4y52p5zw9h8mhgjyrbca8fpyhnxkkasq141a264"))))
> +    (build-system gnu-build-system)
> +    (inputs (list sdl sdl-mixer))

Nitpick: I moved inputs after arguments… out of habit.

> +    (arguments
> +     (list
> +      #:tests? #f  ; no tests
> +      #:phases
> +      #~(modify-phases %standard-phases
> +          (add-after 'unpack 'fix-sdl-paths
> +            (lambda _
> +              (let ((share (string-append #$output "/share")))
> +                ;; fix name and append path to SDL_mixer.h

Nitpick: I used proper capitalization and typography (missing final
full stop).

> +                (substitute* "src/Makefile"
> +                  (("GAME_NAME=alienBlaster")
> +                   "GAME_NAME=alienblaster")
> +                  (("SDL_FLAGS=\\$\\(shell sdl-config --cflags\\)" line)
> +                   (string-append line " -I" #$sdl-mixer "/include/SDL")))

I replace #$sdl-mixer with #$(this-package-inputs "sdl-mixer") for
compatibility with package transformations.

> +                ;; substitute relative paths in .cfg and source/header files

Nitpick: Here too, I slightly reformatted the comment.

> +                (substitute* (find-files "./cfg")
> +                  (("(\\./)?images") (string-append share "/images")))
> +                (substitute* (list "src/global.h" "src/global.cc")
> +                  (("./images") (string-append share "/images"))
> +                  (("./sound") (string-append share "/sound"))
> +                  (("./cfg") (string-append share "/cfg"))))))
> +
> +          (delete 'configure)
> +
> +          (replace 'install
> +            (lambda _
> +              (let ((bin (string-append #$output "/bin")))
> +                (install-file "alienblaster" bin)
> +                (for-each
> +                 (lambda (dir)
> +                   (copy-recursively dir (string-append #$output "/share/" dir)))
> +                 '("images" "sound" "cfg")))
> +              #t)))))

I removed the trailing #T, which is not necessary in new package style.

Regards,
-- 
Nicolas Goaziou