| Message ID | cover.1635590221.git.public@yoctocell.xyz |
|---|---|
| Headers | show |
| Series | Fixes to ‘guix home import’ | expand |
Hi! Xinglu Chen <public@yoctocell.xyz> skribis: > guix home: import: Make the user to specify a destination directory. > guix home: import: Allow multiple modules to be imported for each > service. > guix home: import: Fix module name for Bash service. > guix home: import: Don’t use 'slurp-file-gexp'. > guix home: import: Delete duplicate modules when importing. > doc: Document the ‘guix home import’ subcommand. > Add tests for ‘guix home import’. > guix home: import: Call ‘local-file’ with ‘name’ argument. That’s a nice improvement. In the interest of moving forward, I applied the whole series and followed up with a few changes: c4ac8cf4f6 doc: Mention 'guix home reconfigure' upfront. 971a69d8e3 doc: Avoid misuse of @ref. 7711a6c3f4 doc: Mention "guix home import" upfront. 6f4ca78761 home: import: Avoid duplication of 'manifest->code'. 96728c54df home: import: Factorize triplicated 'version-spec' procedure. f3933ae40d home: import: Clarify "destination directory". 341fba217f home: import: Compare procedures with 'eq?'. Part of it is about removing duplicated code, in particular ‘manifest->code’. It’s important to factorize non-trivial code like this. The last commits improve documentation so users learn about ‘guix home import’ when they get started. It’s really nice to have this tool! I find it perhaps a bit confusing to have to specify a target directory to ‘guix home import’; simply trying to document it shows that it’s non-obvious. I wonder if the argument should be optional (in which case the files wouldn’t be copied). But then people are likely to run into the problems this addresses. Or perhaps it would be more consistent to have: guix home import ~/foo create ~/foo/config.scm, instead of printing it to stdout? The documentation would be clearer: “populate ~/foo with all the configuration files of your home environment.” Thoughts? Ludo’.
Hi, On Sun, Oct 31 2021, Ludovic Courtès wrote: > Hi! > > Xinglu Chen <public@yoctocell.xyz> skribis: > >> guix home: import: Make the user to specify a destination directory. >> guix home: import: Allow multiple modules to be imported for each >> service. >> guix home: import: Fix module name for Bash service. >> guix home: import: Don’t use 'slurp-file-gexp'. >> guix home: import: Delete duplicate modules when importing. >> doc: Document the ‘guix home import’ subcommand. >> Add tests for ‘guix home import’. >> guix home: import: Call ‘local-file’ with ‘name’ argument. > > That’s a nice improvement. In the interest of moving forward, I applied > the whole series and followed up with a few changes: > > c4ac8cf4f6 doc: Mention 'guix home reconfigure' upfront. > 971a69d8e3 doc: Avoid misuse of @ref. > 7711a6c3f4 doc: Mention "guix home import" upfront. > 6f4ca78761 home: import: Avoid duplication of 'manifest->code'. > 96728c54df home: import: Factorize triplicated 'version-spec' procedure. > f3933ae40d home: import: Clarify "destination directory". > 341fba217f home: import: Compare procedures with 'eq?'. > > Part of it is about removing duplicated code, in particular > ‘manifest->code’. It’s important to factorize non-trivial code like > this. > > The last commits improve documentation so users learn about ‘guix home > import’ when they get started. Thanks for taking care of this! > It’s really nice to have this tool! I find it perhaps a bit confusing > to have to specify a target directory to ‘guix home import’; simply > trying to document it shows that it’s non-obvious. > > I wonder if the argument should be optional (in which case the files > wouldn’t be copied). But then people are likely to run into the > problems this addresses. Yeah, that could be a bit confusing for people. > Or perhaps it would be more consistent to have: > > guix home import ~/foo > > create ~/foo/config.scm, instead of printing it to stdout? > > The documentation would be clearer: “populate ~/foo with all the > configuration files of your home environment.” Thoughts? I originally made it to print to stdout because that’s what the ‘guix import’ commands do, but it could make sense to just populate a file directly. I don’t really have a strong opinion on this, so if nobody has any objections, I could send a patch for this. :-)
On 2021-10-31 18:38, Xinglu Chen wrote: > Hi, > > On Sun, Oct 31 2021, Ludovic Courtès wrote: > >> Hi! >> >> Xinglu Chen <public@yoctocell.xyz> skribis: >> >>> guix home: import: Make the user to specify a destination directory. >>> guix home: import: Allow multiple modules to be imported for each >>> service. >>> guix home: import: Fix module name for Bash service. >>> guix home: import: Don’t use 'slurp-file-gexp'. >>> guix home: import: Delete duplicate modules when importing. >>> doc: Document the ‘guix home import’ subcommand. >>> Add tests for ‘guix home import’. >>> guix home: import: Call ‘local-file’ with ‘name’ argument. >> >> That’s a nice improvement. In the interest of moving forward, I applied >> the whole series and followed up with a few changes: >> >> c4ac8cf4f6 doc: Mention 'guix home reconfigure' upfront. >> 971a69d8e3 doc: Avoid misuse of @ref. >> 7711a6c3f4 doc: Mention "guix home import" upfront. >> 6f4ca78761 home: import: Avoid duplication of 'manifest->code'. >> 96728c54df home: import: Factorize triplicated 'version-spec' procedure. >> f3933ae40d home: import: Clarify "destination directory". >> 341fba217f home: import: Compare procedures with 'eq?'. >> >> Part of it is about removing duplicated code, in particular >> ‘manifest->code’. It’s important to factorize non-trivial code like >> this. >> >> The last commits improve documentation so users learn about ‘guix home >> import’ when they get started. > > Thanks for taking care of this! > >> It’s really nice to have this tool! I find it perhaps a bit confusing >> to have to specify a target directory to ‘guix home import’; simply >> trying to document it shows that it’s non-obvious. >> >> I wonder if the argument should be optional (in which case the files >> wouldn’t be copied). But then people are likely to run into the >> problems this addresses. > > Yeah, that could be a bit confusing for people. > >> Or perhaps it would be more consistent to have: >> >> guix home import ~/foo >> >> create ~/foo/config.scm, instead of printing it to stdout? It's a good idea. home.scm or home-config.scm can be another option to make it clearer that it's not a system config. >> >> The documentation would be clearer: “populate ~/foo with all the >> configuration files of your home environment.” Thoughts? > > I originally made it to print to stdout because that’s what the ‘guix > import’ commands do, but it could make sense to just populate a file > directly. I don’t really have a strong opinion on this, so if nobody > has any objections, I could send a patch for this. :-)
Hi, Andrew Tropin <andrew@trop.in> skribis: >>> Or perhaps it would be more consistent to have: >>> >>> guix home import ~/foo >>> >>> create ~/foo/config.scm, instead of printing it to stdout? > > It's a good idea. home.scm or home-config.scm can be another option to > make it clearer that it's not a system config. Yes, or even ‘home-configuration.scm’, in keeping with the no-abbreviation policy. :-) If Xinglu or someone else wants to make this change, I’ll happily review/apply! Thanks, Ludo’.
Changes since v2: * Call ‘local-file’ with a ‘name’ argument to make the /gnu/store/… file name more readable. * Import some missing modules. * Remove the ‘%destination-directory’ parameter, and instead pass the directory to ‘generate-bash-configuration+modules’. * Other minor cleanups. Xinglu Chen (8): guix home: import: Make the user to specify a destination directory. guix home: import: Allow multiple modules to be imported for each service. guix home: import: Fix module name for Bash service. guix home: import: Don’t use 'slurp-file-gexp'. guix home: import: Delete duplicate modules when importing. doc: Document the ‘guix home import’ subcommand. Add tests for ‘guix home import’. guix home: import: Call ‘local-file’ with ‘name’ argument. Makefile.am | 1 + doc/guix.texi | 32 +++++++ guix/scripts/home.scm | 24 +++-- guix/scripts/home/import.scm | 137 ++++++++++++++++---------- tests/home-import.scm | 180 +++++++++++++++++++++++++++++++++++ 5 files changed, 313 insertions(+), 61 deletions(-) create mode 100644 tests/home-import.scm base-commit: c6adc0947396daa6d85ab08837f9cbc86f4d8722