[bug#70158] [PATCH 0/6] Scilab: Fixup install of local Scilab packages.

[Nicolas Graves]

On 2024-04-08 22:31, David Elsing wrote:

> Hi Nicolas,
>
> I'm not really familiar with Scilab itself, I worked on the package to
> update the SuiteSparse dependency and noticed that many files were
> autogenerated.

Thanks for your review David.
>
> Here are some comments about your patches:
> - I think patches 1, 2 and 4 can be combined, as well as patches 3, 5
>   and 6, or was there a reason to split them?

6 is separate from 3 and 5, I'll merge the other though, thanks!

> - For patch 4, I don't think you need to mention how many lines are
>   saved in the commit message. :)

DONE

> - For patches 5 and 6, my impression was that patch files should have a
>   comment and mention the upstream status if relevant. Maybe you can
>   additionally make a merge request upstream for them, as they do not
>   seem specific to Guix? Patch files should also be registered in
>   gnu/local.mk.

I will. 

> - `guix lint` mentions that "bash-minimal" should be in 'inputs' when
>   'wrap-program' is used. This is relevant for cross builds, but at the
>   moment, dune-build-system (for OCaml packages) does not support cross
>   builds anyway. It wouldn't hurt to add bash-minimal anyway though.

Thanks, will add that too.

>
> Otherwise, they look fine to me.
>
> Cheers,
> David

-- 
Best regards,
Nicolas Graves