[bug#34830] Patches for ruby.scm

[Ricardo Wurmus]

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 <address@hidden>
> 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 <address@hidden>
> 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 <address@hidden>
>  ;;; Copyright © 2018 Alex Vong <address@hidden>
>  ;;; Copyright © 2019 Pierre Neidhardt <address@hidden>
> +;;; Copyright © 2019 Keerthi Kota <address@hidden>

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