On 2021-10-31 18:38, Xinglu Chen wrote: > Hi, > > On Sun, Oct 31 2021, Ludovic Courtès wrote: > >> Hi! >> >> Xinglu Chen 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. :-) -- Best regards, Andrew Tropin