[bug#40495] v3-- fixed indentation

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Vitaliy Shatrov via Guix-patches via <guix-patches@gnu.org> writes:

> Subject: [PATCH] gnu: Add taisei, and spirv-cross
>
> * gnu/packages/games.scm  (taisei):      new variable
> * gnu/packages/vulkan.scm (spirv-cross): new variable

It works nicely, thank you.

> +     `(;;configure option developer=true enables the diagnostics
> +       ;;needed for bug reports.  it is 'true' if not "release"
> +       ;;#:build-type "release"

So, what do you suggest here? Use release or developer build?

> +    (synopsis "Fangame and libre clone of Touhou Project")

I would suggest something like

  Shoot'em up game set in a world full of Japanese folklore

> +    (description
> +     "Taisei is a shoot-em-up game: The player controls a character (one of
> +three: Good, Bad, and Dead), dodges the missiles (lots of it cover the screen,
> +but the character's hitbox is very small), and shoot at the adversaries that
> +keep appear on the screen.")


> +    (license (list license:expat      ;game
> +                   license:cc-by4.0   ;resources/00-taisei.pkgdir/bgm/
> +                                        ;atlas/portraits/
> +                   ;;miscellaneous
> +                   license:cc0
> +                   license:public-domain))))

It would be clearer to explain in a comment above the license field what
is subject to what terms.

> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/KhronosGroup/SPIRV-Cross")
> +             (commit (string-append version))))

You can remove the `string-append' here.

> +       (sha256
> +        (base32
> +         "0489s29kqgq20clxqg22y299yxz23p0yjh87yhka705hm9skx4sa"))
> +       (file-name (git-file-name name version))))
> +    (build-system cmake-build-system)
> +    (arguments
> +     `(#:tests? #f  ;FIXME: Tests fail.

IMO, this is not a blocker. However, if you have more information than
"Tests fail", it would be nice to add it in a comment.

> +       #:configure-flags
> +       (list "-DSPIRV_CROSS_SHARED=YES")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'fix-tests-to-find-deps
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (substitute* "CMakeLists.txt"
> +               (((string-append "PATHS "
> +                                "\\$\\{CMAKE_CURRENT_SOURCE_DIR\\}"
> +                                "/external/glslang-build/output/bin"))

Why do you need this? What about simply writing the full string without
`string-append'?

> +             (substitute* "CMakeLists.txt"
> +               (((string-append "PATHS "
> +                                "\\$\\{CMAKE_CURRENT_SOURCE_DIR\\}"
> +                                "/external/spirv-tools-build/output/bin"))

Ditto.

> +    (native-inputs `(("glslang" ,glslang)
> +                     ("python" ,python)
> +                     ("spirv-headers" ,spirv-headers)
> +                     ("spirv-tools" ,spirv-tools)))

Nitpick: I would move the inputs below the `native-inputs' line.

> +    (description
> +     "SPIRV-Cross tries hard to emit readable and clean output from the
> +SPIR-V.  The goal is to emit GLSL or MSL that looks like it was written by a
> +human and not awkward IR/assembly-like code.  NOTE: Individual features are

You can drop the "NOTE:" prefix. 

Actually, I think you can drop everything after "NOTE:". Is it useful
information for someone looking at the package?

> +expected to be mostly complete, but it is possible that certain obscure GLSL
> +features are not yet supported.  However, most missing features are expected
> +to be \"trivial\" improvements at this stage.")

If you disagree with my suggestion, you need to remove these double
quotes. Texinfo uses ``trivial'', but you could also write
@emph{trivial}.

Regards,

-- 
Nicolas Goaziou