Hi Vincent, thanks for the contribution.
On 26/03/16 15:49, vinc...@cloutier.co wrote:
[..]
First some comments on the patch. In general we try to keep line lengths
below 80 chars, and not leave blank lines within package definitions.
(define-public wayback-machine-downloader
(package
(name "wayback-machine-downloader")
(version "0.2.1")
(source
(origin
(method url-fetch)
(uri (rubygems-uri
"wayback_machine_downloader"
version))
Please put the above 3 lines on the same line.
(sha256
(base32
"1nrwm5bc7vqm02m2x0lylxyya446kg0spg6ksi7dfkrad0l9jq8y"))))
(build-system ruby-build-system)
(arguments
`(#:tests? #f ; no rakefile
))
Stylistically, we don't leave empty braces alone like this.
It seems there is a Rakefile with a test task defined in the github
repository, but not in the gem itself. So in this case I would point
guix to download the source from the github page not rubygems.org, as is
currently done for ruby-nokogumbo for example:
(define-public ruby-nokogumbo
(package
(name "ruby-nokogumbo")
(version "1.4.6")
(source (origin
;; We use the git reference, because there's no Rakefile
in the
;; published gem and the tarball on Github is outdated.
(method git-fetch)
(uri (git-reference
(url "https://github.com/rubys/nokogumbo.git";)
(commit "d56f954d20a")))
(file-name (string-append name "-" version "-checkout"))
(sha256
(base32
"0bnppjy96xiadrsrc9dp8y6wvdwnkfa930n7acrp0mqm4qywl2wl"))))
(native-inputs
`(("ruby-rake-compiler" ,ruby-rake-compiler)
Is this really needed?
("ruby-minitest" ,ruby-minitest)))
(synopsis
"Download website from archive.org's Wayback Machine")
Maybe "Download a website ..." ? Also, please put on one line.
(description
"Download any website from the Wayback Machine. Wayback Machine
by Internet Archive (archive.org) is an awesome tool to view any
website at any point of time but lacks an export feature. Wayback
Machine Downloader brings exactly this.")
I would remove "awesome" as being too opinionated. I think it would also
make sense to comment on the fact that it is a command line tool.
(home-page
"https://github.com/hartator/wayback-machine-downloader";)
(license expat)))
Then I ran: `guix package -i wayback-machine-downloader -f
~/Documents/ruby` and it successfully installed.
So my questions are:
1) Do you guys and gals have a better workflow that includes the git
repo, so I can send a patch? All I saw in the documentation was about
building guix itself. I guess I could clone somewhere and use `guix
package -f`, but will this be a reliable way of testing? And will
this make my guix less stable on the long run?
My personal workflow for this situation would be something like this:
$ git checkout -b wayback-machine-downloader origin/master
$ make -j4
... modify gnu/packages/ruby.scm adding the package definition ...
$ ./pre-inst-env guix build -K wayback-machine-downloader
... modify further as necessary until I am happy ...
... run through the checklist of things to do (e.g. list) when
submitting a package at
https://www.gnu.org/software/guix/manual/guix.html#Submitting-Patches
$ git diff #check that I'm committing the right code and only the one
package's code
$ git commit
... edit commit message ...
$ git format-patch -1
... send to guix-devel ...
hope that helps.
2) Should I add "ruby-" before the name of the package? I know
technically all gems should have "ruby-" before the name, but this is
designed to be use independently. Could it have multiple names, or is
it a bad idea?
In this case I would say no, leave it as wayback-machine-downloader
since it is not intended to be used as a library, rather a command line
tool.
3) Where does this package belong in the directory?
My guess would be web.scm not ruby.scm.
4) Is the package declaration itself all right? Are packages sorted or
organized in any way?
This depends on the file being modified, in ruby.scm we just add it to
the end of the file.
5) I speak fluent French, can I add a description and summary in French?
I'm not sure on this sorry.
Would you mind submitting an updated patch please?
Thanks,
ben
