Sree Harsha Totakura <sreehar...@totakura.in> skribis: > * guix/svn-download.scm, guix/build/svn.scm: New files. > * Makefile.am (MODULES): Add them.
Looks good to me! Some comments: > +(define* (svn-fetch url revision directory > + #:key (svn-command "svn")) > + "Fetch REVISION from URL into DIRECTORY. REVISION must be a valid svn > +revision. Return #t on success, #f otherwise." ‘revision’ can/should be a number, no? Please augment the docstring to say that, and... > + (and (zero? (system* svn-command "checkout" "--non-interactive" > + ;; Trust the server certificate. This is OK as we > + ;; verify the checksum later. This can be removed > when > + ;; ca-certificates package is added. > + "--trust-server-cert" "-r" revision url directory)) ... possibly use (number->string revision) here ↑. > +;;; Commentary: > +;;; > +;;; An <origin> method that fetches a specific revision from a Subversion > +;;; repository. The repository URL and revision are specified with a > +;;; <svn-reference> object. > +;;; > +;;; Code: > +(define-record-type* <svn-reference> > + svn-reference make-svn-reference > + svn-reference? > + (url svn-reference-url) > + (revision svn-reference-revision)) Please align things, add a comment saying whether ‘revision’ is a number or string, and add a newline before ‘define-record-type*’. Thanks! Ludo’.
