[bug#34830] Patches for ruby.scm

[Ricardo Wurmus <rekado@elephly.net>]

Hi Keerthi,

>> I am Keerthi kota . I wrote two package named ruby-bounce and
>> ruby-skinny-jeans

Thank you!  This seems to be your first patch submission to Guix.  Thank
you for taking the time to package something and sharing the results
with us!

I have a couple of comments about the patches that I hope will be useful
for your future contributions.

> From 596a48d092b6450eba7a965ee23f1cfb6309ba4b Mon Sep 17 00:00:00 2001
> From: Keerthi Kota <kota.keerthi1998@gmail.com>
> Date: Mon, 11 Mar 2019 18:51:49 +0530
> Subject: [PATCH] gnu: Add ruby-bounce.
>
>         * gnu/packages/ruby.scm (ruby-bounce): New variable.

Please don’t indent the commit message.

>
> +(define-public ruby-bounce
> +  (package
> +    (name "ruby-bounce")
> +    (version "0.1.2")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (rubygems-uri "bounce" version))
> +       (sha256
> +        (base32
> +         "1rlafvk1h5pb9k16lslqrfmfv6rl0hrskkhlliifd9dbm9s54cqg"))))
> +    (build-system ruby-build-system)
> +    (arguments
> +     '(;; No included tests
> +       #:tests? #f))
> +

I would prefer not to have this empty line here.

> +    (propagated-inputs
> +     `(("ruby-activerecord" ,ruby-activerecord)))
> +    (synopsis
> +     "Bounce will save and return an active record object. This results in a nice refactor of update and create actions in your controllers when used with respond_with.")

This is a *very* long synopsis :) If you take a look at other packages
you will see that a synopsis is to be a very short, general explanation
of what the package is about, without going into any detail.

> +    (description "bounce will save and return an active record object.  This results in a nice refactor of update and create actions in your controllers when used with respond_with.")

This is a very long line.  We suggest running the linter before
submitting a patch, because it will give you a couple of hints:

    ./pre-inst-env guix lint ruby-bounce

> +    (home-page
> +     "http://github.com/johnnytommy/bounce")

Please use https here and join these two lines.  Curiously this URL
cannot be found.  Is there another home page?

> +    (license license:expat)))

I haven’t checked this yet.

> From 0742fb40e7742349ce8b0e346c403cd7d85857c5 Mon Sep 17 00:00:00 2001
> From: Keerthi Kota <kota.keerthi1998@gmail.com>
> Date: Mon, 11 Mar 2019 18:43:25 +0530
> Subject: [PATCH]     gnu: Add ruby-skinny-jeans.
>
>     * gnu/packages/ruby.scm (ruby-skinny-jeans): New variable.

Same as above; please don’t indent these lines.

>  gnu/packages/ruby.scm | 2262 +++++++++++++++++++++++++------------------------
>  1 file changed, 1146 insertions(+), 1116 deletions(-)
>
> diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
> index 818553848..e6e0f09c2 100644
> --- a/gnu/packages/ruby.scm
> +++ b/gnu/packages/ruby.scm
> @@ -14,6 +14,7 @@
>  ;;; Copyright © 2018 Vasile Dumitrascu <va511e@yahoo.com>
>  ;;; Copyright © 2018 Alex Vong <alexvong1995@gmail.com>
>  ;;; Copyright © 2019 Pierre Neidhardt <mail@ambrevar.xyz>
> +;;; Copyright © 2019 Keerthi Kota <kota.keerthi1998@gmail.com>

Good! :)

>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -89,23 +90,23 @@
>      (build-system gnu-build-system)
>      (arguments
>       `(#:test-target "test"
> -       #:phases
> -       (modify-phases %standard-phases
> -         (add-before 'configure 'replace-bin-sh-and-remove-libffi
> -           (lambda _
…

You accidentally included hundreds of lines of unrelated changes in your
patch.

> +
> +(define-public ruby-skinny-jeans
> +  (package
> +    (name "ruby-skinny-jeans")
> +    (version "0.2.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (rubygems-uri "skinny-jeans" version))
> +       (sha256
> +        (base32
> +         "0lbhrkigypxikgzfzplsffilrpkym0snkznlqlba024y5m37w70m"))))
> +    (build-system ruby-build-system)
> +    (arguments
> +     '(;; No included tests
> +       #:tests? #f))
> +

Please remove this extra line.

> +    (propagated-inputs
> +     `(("ruby-activerecord" ,ruby-activerecord)
> +       ("ruby-sqlite3" ,ruby-sqlite3)))
> +    (synopsis
> +     "Fast webserver log parser for persisting daily pageviews per path to sqlite")
> +    (description
> +     "Fast webserver log parser for persisting daily pageviews per path to sqlite")

Please update both synopsis and description.  The synopsis should be
shorter and the description should consist of complete sentences.
Please also break long lines — guix lint will tell you when a line is
too long.

> +    (home-page
> +     "http://github.com/jotto/skinny-jeans")

Same as above: please use https and merge these two lines.  As before
this URL also does not exist any more.  Is there another home page?

> +    (license license:expat)))
> +
> +

I haven’t checked the license yet.  Please remove the extra empty lines
before and after the package definition.

Could you please send revised patches as a reply to this message?

Thanks again!

--
Ricardo