On Thu, 28 Sep 2017 18:38:19 +1000
Ben Woodcroft <b.woodcroft@uq.edu.au> wrote:

> On 25/09/17 03:17, Christopher Baines wrote:
> > * gnu/packages/databases.scm (es-dump-restore): New variable.  
> [..]
> > +    (arguments
> > +     '(#:tests? #f  
> Including a reason here would be good.

Done, turns out the reason is that there is no testsuite.

> > +       #:phases
> > +       (modify-phases %standard-phases
> > +         (add-after 'install 'wrap-bin-es_dump_restore
> > +           (lambda* (#:key outputs #:allow-other-keys)
> > +             (wrap-program (string-append (assoc-ref outputs "out")
> > +                                          "/bin/es_dump_restore")
> > +               `("GEM_PATH" ":" prefix (,(string-append
> > +                                          (getenv "GEM_PATH")
> > +                                          ":"
> > +                                          (getenv "GEM_HOME")))))
> > +             #t)))))  
> Again, it would be preferable to have this done as part of the build 
> system, but OK.
> 
> > +    (propagated-inputs
> > +     `(("ruby-httpclient" ,ruby-httpclient)
> > +       ("ruby-multi-json" ,ruby-multi-json)
> > +       ("ruby-progress_bar" ,ruby-progress_bar)
> > +       ("ruby-rubyzip" ,ruby-rubyzip)
> > +       ("ruby-thor" ,ruby-thor)))
> > +    (synopsis
> > +     "Utility for dumping and restoring ElasticSearch indexes")  
> This can be moved onto the previous line.
> 
> > +    (description
> > +     "This package provides a utility for dumping the contents of
> > an +ElasticSearch index to a compressed file and restoring the
> > dumpfile back to an +ElasticSearch server")
> > +    (home-page
> > +     "https://github.com/patientslikeme/es_dump_restore")  
> As can this. Otherwise LGTM - thanks for this series.

These two formatting issues should be fixed now. Thanks for the review
and patches Ben. I've sent an updated set of patches.