https://bugzilla.redhat.com/show_bug.cgi?id=990932
Bug ID: 990932 Summary: Review Request: erlang-riaknostic - A diagnostic tool for Riak installations Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: lemenkov@gmail.com QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
Spec URL: http://peter.fedorapeople.org/erlang-riaknostic.spec SRPM URL: http://peter.fedorapeople.org/erlang-riaknostic-1.1.0-1.fc20.src.rpm Description: A set of tools that diagnoses common problems which could affect a Riak node or cluster. When experiencing any problem with Riak, riaknostic should be the first thing run during troubleshooting. The tool is integrated with Riak via the riak-admin script. Fedora Account System Username: peter
This is one of the requirements for Riak 1.3.2+.
Koji scratchbuild for Rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5687654
https://bugzilla.redhat.com/show_bug.cgi?id=990932
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zbyszek@in.waw.pl Assignee|nobody@fedoraproject.org |zbyszek@in.waw.pl
https://bugzilla.redhat.com/show_bug.cgi?id=990932
--- Comment #1 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- Hi Peter, I found some small issues:
rpmlint output:
erlang-riaknostic.src: W: spelling-error %description -l en_US riak → false positive erlang-riaknostic.src: W: invalid-url Source0: basho-riaknostic-v1.1.0-0-g8cfb08e.tar.gz → You can use github link like https://github.com/basho/riaknostic/archive/v1.1.0.tar.gz, which should give the same file.
Also, there's a new upstream release v1.2.0.
erlang-riaknostic.x86_64: W: spelling-error %description -l en_US riak → false positive
erlang-riaknostic.x86_64: E: no-binary erlang-riaknostic.x86_64: W: only-non-binary-in-usr-lib → both are normal for Erlang-related packages
Is the package really usable without riak-admin? The documentation mentions that it can be run standalone for testing purposes... I don't know almost anything about riak, so I'm guessing here, but I think that it should have Requires:riak.
Also, maybe the description could be updated to say "is a plugin for riak-admin" instead of "is integrated with ...", just to be more direct.
So, the only two non-suggestion things are Source and the new upstream release. I'll do a point-by-point review when that is fixed.
https://bugzilla.redhat.com/show_bug.cgi?id=990932
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #2 from Christopher Meng cickumqt@gmail.com --- Should we add VCS tag? I think rpm doesn't support checkout from git and install, not sure if we need that.
https://bugzilla.redhat.com/show_bug.cgi?id=990932
--- Comment #3 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- (In reply to Christopher Meng from comment #2)
Should we add VCS tag? I think rpm doesn't support checkout from git and install, not sure if we need that.
I don't think a tag is needed. rpm just uses the provided tarball, and doesn't use the URL in Source directly. github allows to download a tarball from any commit, tagged or not.
https://bugzilla.redhat.com/show_bug.cgi?id=990932
--- Comment #4 from Peter Lemenkov lemenkov@gmail.com --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
Hi Peter, I found some small issues:
rpmlint output:
erlang-riaknostic.src: W: spelling-error %description -l en_US riak → false positive erlang-riaknostic.src: W: invalid-url Source0: basho-riaknostic-v1.1.0-0-g8cfb08e.tar.gz → You can use github link like https://github.com/basho/riaknostic/archive/v1.1.0.tar.gz, which should give the same file.
Also, there's a new upstream release v1.2.0.
erlang-riaknostic.x86_64: W: spelling-error %description -l en_US riak → false positive
erlang-riaknostic.x86_64: E: no-binary erlang-riaknostic.x86_64: W: only-non-binary-in-usr-lib → both are normal for Erlang-related packages
Is the package really usable without riak-admin? The documentation mentions that it can be run standalone for testing purposes... I don't know almost anything about riak, so I'm guessing here, but I think that it should have Requires:riak.
Also, maybe the description could be updated to say "is a plugin for riak-admin" instead of "is integrated with ...", just to be more direct.
So, the only two non-suggestion things are Source and the new upstream release. I'll do a point-by-point review when that is fixed.
https://bugzilla.redhat.com/show_bug.cgi?id=990932
--- Comment #5 from Peter Lemenkov lemenkov@gmail.com --- Sorry, submitted too fast.
(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
erlang-riaknostic.src: W: invalid-url Source0: basho-riaknostic-v1.1.0-0-g8cfb08e.tar.gz → You can use github link like https://github.com/basho/riaknostic/archive/v1.1.0.tar.gz, which should give the same file.
Fixed.
Also, there's a new upstream release v1.2.0.
Yes, I'm aware of this. But I can't use 1.2.0 until I upgrade to Riak 1.4.x (I plan to update in Fedora 21).
Is the package really usable without riak-admin? The documentation mentions that it can be run standalone for testing purposes... I don't know almost anything about riak, so I'm guessing here, but I think that it should have Requires:riak.
It's not a standalone package - it's a library. README doesn't properly describes that.
riak-admin is the script from riak package which could use riaknostic if this library is available. So it's riak who is dependent upon riaknostic, not the opposite.
Also, maybe the description could be updated to say "is a plugin for riak-admin" instead of "is integrated with ...", just to be more direct.
Yes, definitely!
New package:
* http://peter.fedorapeople.org/erlang-riaknostic.spec * http://peter.fedorapeople.org/erlang-riaknostic-1.1.0-1.fc20.src.rpm
(In reply to Christopher Meng from comment #2)
Should we add VCS tag? I think rpm doesn't support checkout from git and install, not sure if we need that.
I added this mostly for the reference.
https://bugzilla.redhat.com/show_bug.cgi?id=990932
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+
--- Comment #6 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- REVIEW:
Legend: + = PASSED, - = FAILED, 0 = Not Applicable
- rpmlint is not completely silent erlang-riaknostic.spec: W: invalid-url Source0: https://github.com/basho/riaknostic/archive/1.1.0/riaknostic-1.1.0.tar.gz HTTP Error 404: Not Found ^^^ I think it should be https://github.com/basho/riaknostic/archive/v1.1.0.tar.gz
0 packages and 1 specfiles checked; 0 errors, 1 warnings.
erlang-riaknostic.x86_64: W: spelling-error %description -l en_US riak -> risk, rial, rink erlang-riaknostic.x86_64: E: no-binary erlang-riaknostic.x86_64: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 1 errors, 2 warnings. ^^^ false positives
+ The package is named according to the Package Naming Guidelines. Package follows erlang package naming guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec. + The package meets the Packaging Guidelines. + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The License field in the package spec file matches the actual license (Apache License v. 2). + The file, containing the text of the license(s) for the package, is included in %doc. + The spec file is written in American English. + The spec file for the package is legible. - The sources used to build the package, match the upstream source, as provided in the spec URL. Please use https://github.com/basho/riaknostic/archive/v1.1.0.tar.gz as the Source.
+ The package successfully compiles and builds into binary rpms on at least one primary architecture. + All build dependencies are listed in BuildRequires. Is the explicit version in 'erlang-erts >= R13B' necessary? Isn't R13B ancient?
0 No need to handle locales. 0 The package doesn't store shared library files. + The package does NOT bundle copies of system libraries. 0 The package is not designed to be relocatable. + The package MUST own all directories that it creates. + The package doesn't list a file more than once in the spec file's %files listings. + Permissions on files are set properly. + The package consistently uses macros. + The package contains code, or permissible content. 0 No extremely large documentation files. + Anything, the package includes as %doc, does not affect the runtime of the application. 0 No static libraries. + The package does NOT contain any .la libtool archives. 0 Not a GUI application. + The package does not own files or directories already owned by other packages. + All filenames in rpm packages are valid UTF-8.
APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=990932
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #7 from Peter Lemenkov lemenkov@gmail.com --- Thanks!
New Package SCM Request ======================= Package Name: erlang-riaknostic Short Description: A diagnostic tool for Riak installations Owners: peter Branches: f19 el6 InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=990932
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=990932
--- Comment #8 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=990932
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Fixed In Version| |erlang-riaknostic-1.1.0-1.f | |c20 Resolution|--- |RAWHIDE Last Closed| |2013-08-06 10:27:36
package-review@lists.fedoraproject.org