https://bugzilla.redhat.com/show_bug.cgi?id=1460662
Bug ID: 1460662 Summary: Review Request: rubygem-mustermann - A library using patterns like regular expressions Product: Fedora Version: rawhide Component: Package Review Assignee: nobody@fedoraproject.org Reporter: jaruga@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://pagure.io/jaruga_rubygem-mustermann/raw/master/f/rubygem-mustermann.... SRPM URL: https://pagure.io/jaruga_rubygem-mustermann/raw/master/f/rubygem-mustermann-... Description: A library using patterns like regular expressions Fedora Account System Username: jaruga
Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=19993792
https://bugzilla.redhat.com/show_bug.cgi?id=1460662
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |vondruch@redhat.com Assignee|nobody@fedoraproject.org |vondruch@redhat.com Flags| |fedora-review?
--- Comment #1 from Vít Ondruch vondruch@redhat.com --- I'll take it for review ...
https://bugzilla.redhat.com/show_bug.cgi?id=1460662
--- Comment #2 from Vít Ondruch vondruch@redhat.com --- * Summary/Description - I'd say that the summary is misleading. The original (but capitalized) would be better IMHO. - The description should start with capital letter.
* Group tag is deprecated - Please remove the Group tag. It is deprecated [1].
* Test {support,mustermann-contrib} libraries - It would be good to document why the {support,mustermann-contrib} is needed (e.g. "Support routines required by test suite"). - I don't think there is any reason to expand these libraries into the .%{gem_instdir}. Although we are using this on several places, this is used typically to mimic the upstream repository layout. In this case, I think it would be much better to expand the sources using the %setup macro in %prep section, e.g.:
~~~ %setup -q -D -T -n %{gem_name}-%{version} -b1 -b2 ~~~
* Idiomatic test suite execution. - We typically execute the RSpec test suite via "rspec spec". If you need to specify additional path, the "rspec" command supports -I variable just as ruby does. E.g. this should work for you:
~~~ rspec -I{support,mustermann-contrib}/lib spec ~~~
[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections
https://bugzilla.redhat.com/show_bug.cgi?id=1460662
--- Comment #3 from Vít Ondruch vondruch@redhat.com --- BTW there appears to be circular dependency with Sinatra, you should consider to prepare Mustermann for bootstrap.
https://bugzilla.redhat.com/show_bug.cgi?id=1460662
--- Comment #4 from Jun Aruga jaruga@redhat.com --- Vit, thank you for the review.
I fixed those items that you mentioned.
1 point.
%setup -q -D -T -n %{gem_name}-%{version} -b1 -b2
Below "-b n" looks better regarding the official document' sample.
~~~ %setup -q -D -T -n %{gem_name}-%{version} -b 1 -b 2 ~~~
http://ftp.rpm.org/max-rpm/s1-rpm-inside-macros.html
%setup -T -b 0
I updated below files.
Spec URL: https://pagure.io/jaruga_rubygem-mustermann/raw/master/f/rubygem-mustermann.... SRPM URL: https://pagure.io/jaruga_rubygem-mustermann/raw/master/f/rubygem-mustermann-...
Could you check again? Thanks.
https://bugzilla.redhat.com/show_bug.cgi?id=1460662
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #5 from Vít Ondruch vondruch@redhat.com --- Just one minor nit. I'd move the seds out of the pushd/popd block, since they are using absolute path now.
Otherwise the package looks good => APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1460662
--- Comment #6 from Gwyn Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/rubygem-mustermann
https://bugzilla.redhat.com/show_bug.cgi?id=1460662
Jun Aruga jaruga@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Fixed In Version| |rubygem-mustermann-1.0.0-1. | |fc27 Resolution|--- |RAWHIDE Last Closed| |2017-06-13 12:47:10
--- Comment #7 from Jun Aruga jaruga@redhat.com --- Thank you. I pushed the files including your additional mentioning item.
package-review@lists.fedoraproject.org