Hi Ricardo, Thanks for the review. Let me know if you prefer a review before commit next time.
Ricardo Wurmus writes: > Ricardo Wurmus <[email protected]> writes: > >> Hi Alex, >> >>> atheia pushed a commit to branch master >>> in repository guix. >>> >>> commit 63bf0afdf6a27702879c12d7d591c46246c7ad36 >>> Author: Alex Sassmannshausen <[email protected]> >>> Date: Fri Feb 15 16:55:42 2019 +0100 >>> >>> gnu: guile-hall: Update to 0.2. >>> >>> * gnu/packages/guile-xyz.scm (guile-hall): Update to 0.2. >> […] >>> - (license license:gpl3+))) >> […] >>> + (license gpl3+))) >> >> I don’t think this is correct. Did you remove the license: prefix by >> mistake? You're right, this is incorrect. One of the points of Hall is to generate reproducible guix packages — but currently they default to using (guix licenses) without prefix. The generated recipe hence has no prefix. I will fix this. > I looked over the rest of the changes and they seem to make the package > definition much more complicated than it needs to be. Is this in > preparation of a bigger change in the near future? Again, this is the result of auto-generation: Hall generically wraps any script files in the scripts directory of a project with all Guile dependencies — rather than the developer having to manually specify these things. In the case of Hall this introduces complexity because it has a simple dependency & script structure. I can see two ways around this: - making hall more intelligent in future and reducing complexity if the dependency requirements are simpler. - hiding the complexity inside a build system for Guix, which carries out the steps currently added by Hall explicitly. What do you think? ---- I can also see that you have fixed my commit and tweaked it in several ways — thank you for doing this. Are the steps you carried out "the recommended way" of doing things now? If so then I can adapt hall to default to this workflow, so we can encourage new contributors to do "the right thing". Cheers for your work! Alex
