Bug 1032186 - Review Request: rubygem-rack-openid - Provides a more HTTPish API around the ruby-openid library
Review Request: rubygem-rack-openid - Provides a more HTTPish API around the ...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-19 11:54 EST by Ken Dreyer
Modified: 2013-12-13 22:28 EST (History)
1 user (show)

See Also:
Fixed In Version: rubygem-rack-openid-1.4.1-2.fc20
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-12-10 21:03:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ken Dreyer 2013-11-19 11:54:26 EST
Spec URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-rack-openid.spec
SRPM URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-rack-openid-1.4.0-1.fc21.src.rpm
Description: Provides a more HTTPish API around the ruby-openid library.
Fedora Account System Username: ktdreyer

F21 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6199009
Comment 1 Ken Dreyer 2013-11-19 12:03:18 EST
(Just a note that this RPM requires rubygem-rots, which is not yet available in the F20 and F19 stable repos: https://admin.fedoraproject.org/updates/rubygem-rots . You can do the review in a Rawhide buildroot until rubygem-rots gets pushed to stable for those branches.)
Comment 2 Ken Dreyer 2013-11-26 22:46:29 EST
(rubygem-rots is in F20 and F19, so rubygem-rack-openid can build on those versions of Fedora without issue.)
Comment 3 Ken Dreyer 2013-11-26 23:52:20 EST
I've updated to the latest upstream version.

* Wed Nov 27 2013 Ken Dreyer <ktdreyer@ktdreyer.com> - 1.4.1-1
- Update to rack-openid 1.4.1
- Ship license file from upstream Git

Spec URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-rack-openid.spec
SRPM URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-rack-openid-1.4.1-1.fc21.src.rpm

F21 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6230770
Comment 4 Mamoru TASAKA 2013-11-27 01:58:37 EST
Taking.

I would appreciate it if you would review one of my review requests (e.g. bug 1035129 )
Comment 5 Ken Dreyer 2013-11-27 03:58:46 EST
Thanks for taking the review.

I just realized that 1.4.1-1 contains a bad copy-and-paste from another gem. This text:

  # Until upstream ships the full text of the ASL 2.0 in a released gem, we will
  # ship the version from Git master.
  install -p -m 0644 %{SOURCE1} %{buildroot}%{gem_instdir}/LICENSE

needs to use %{SOURCE3} instead of %{SOURCE1}, and the comment needs to say "MIT", rather than "ASL 2.0". Whoops! I can fix this after your full review.
Comment 6 Mamoru TASAKA 2013-11-27 23:31:03 EST
Some other comments:

* Unpacking tarball in %prep
----------------------------------------------------
    49  %setup -q -D -T -n %{gem_name}-%{version}
    53  tar -xJf %{SOURCE2}
----------------------------------------------------
  - I guess "%setup -q -D -T -n %{gem_name}-%{version} -a 2" is
    better.

* For Source1
  - It is better to add "set -e"
  - And I would remove rack-openid-$VERSION.zip (not a blocker)
  - Also it is better to suppress this rpmlint:
-----------------------------------------------------
rubygem-rack-openid.src: W: strange-permission rubygem-rack-openid-generate-test-tarball.sh 0775L
-----------------------------------------------------

Please check the comments above, and also your previous comments.
Comment 7 Ken Dreyer 2013-11-28 10:16:51 EST
Thanks for the suggestions. I've fixed all of them except the strange-permission error in rpmlint. I did change it from 775 to 755, but rpmlint is still unhappy that the permissions are not 644. To me it makes sense to keep the script executable. 

* Thu Nov 28 2013 Ken Dreyer <ktdreyer@ktdreyer.com> - 1.4.1-2
- Address issues from package review (RHBZ #1032186)
- Correct comment about LICENSE
- Unpack the tests tarball during %%setup
- Use "set -e" in tests tarball generation script
- Clean up zip file in tests tarball generation script

Exact changes in Git: http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-rack-openid.git/commit/?id=ca565b5b5a4322bcb8df6ae616055feb7cf46db6

Spec URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-rack-openid.spec
SRPM URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-rack-openid-1.4.1-2.fc21.src.rpm

F21 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6235787
Comment 8 Mamoru TASAKA 2013-11-30 12:39:47 EST
Approving.

----------------------------------------------------
  This package (rubygem-rack-openid) is APPROVED
  by mtasaka
----------------------------------------------------
Comment 9 Ken Dreyer 2013-11-30 18:21:10 EST
Thanks Mamoru for the review!

New Package SCM Request
=======================
Package Name: rubygem-rack-openid
Short Description: Provides a more HTTPish API around the ruby-openid library
Owners: ktdreyer
Branches: f19 f20
Comment 10 Gwyn Ciesla 2013-12-01 16:46:18 EST
Git done (by process-git-requests).
Comment 11 Fedora Update System 2013-12-01 21:39:26 EST
rubygem-rack-openid-1.4.1-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/rubygem-rack-openid-1.4.1-2.fc20
Comment 12 Fedora Update System 2013-12-01 21:40:03 EST
rubygem-rack-openid-1.4.1-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/rubygem-rack-openid-1.4.1-2.fc19
Comment 13 Fedora Update System 2013-12-02 18:51:33 EST
rubygem-rack-openid-1.4.1-2.fc20 has been pushed to the Fedora 20 testing repository.
Comment 14 Fedora Update System 2013-12-10 21:03:19 EST
rubygem-rack-openid-1.4.1-2.fc19 has been pushed to the Fedora 19 stable repository.
Comment 15 Fedora Update System 2013-12-13 22:28:40 EST
rubygem-rack-openid-1.4.1-2.fc20 has been pushed to the Fedora 20 stable repository.

Note You need to log in before you can comment on or make changes to this bug.