Bug 502490 - Review Request: rubygem-state_machine - Adds support for creating state machines for attributes on any Ruby class
Review Request: rubygem-state_machine - Adds support for creating state machi...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michal Ingeli
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-25 09:42 EDT by Darryl L. Pierce
Modified: 2015-06-21 20:06 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-14 08:57:10 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mi: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Darryl L. Pierce 2009-05-25 09:42:36 EDT
Spec URL: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine.spec
SRPM URL: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-1.fc10.src.rpm
Description: Adds support for creating state machines for attributes on any Ruby class
Comment 1 Michal Ingeli 2009-06-30 19:30:04 EDT
- bad license. should be MIT [1]
- missing require for ruby(abi)
- file listed twice
  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4/LICENSE
- rubygem-state_machine.src: W: mixed-use-of-spaces-and-tabs (spaces: line 33, tab: line 7)

You can inspire by review made by Mamoru Tasaka for recent ruby gem review [2].

[1] https://fedoraproject.org/wiki/Licensing/MIT#Modern_Style_with_sublicense
[2] https://bugzilla.redhat.com/show_bug.cgi?id=504469
Comment 2 Darryl L. Pierce 2009-07-01 09:07:30 EDT
(In reply to comment #1)
> - bad license. should be MIT [1]

Fixed.

> - missing require for ruby(abi)

Fixed.

> - file listed twice
>   warning: File listed twice:
> /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4/LICENSE
> - rubygem-state_machine.src: W: mixed-use-of-spaces-and-tabs (spaces: line 33,
> tab: line 7)

Hmmm. I don't see this error. I did a scratch build, then downloaded and ran rpmlint on the RPM and saw no complaints. Can you tell me how you produced this?

(mcpierce@mcpierce-laptop:rubygem-state_machine)$ rpmlint ~/Download/rubygem-state_machine-0.7.4-2.fc11.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.


Updated spec: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine.spec
Updated SRPM: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-2.fc11.noarch.rpm
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1446813
Comment 3 Michal Ingeli 2009-07-09 15:47:01 EDT
(In reply to comment #2)
> (In reply to comment #1)
> > - bad license. should be MIT [1] 
> Fixed.

OK

> > - missing require for ruby(abi)
> Fixed. 

OK

> > - file listed twice
> >   warning: File listed twice:
> > /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4/LICENSE

You can see this warning at the end of build.log [1].

> > - rubygem-state_machine.src: W: mixed-use-of-spaces-and-tabs (spaces: line 33,
> > tab: line 7)
> 
> Hmmm. I don't see this error. I did a scratch build, then downloaded and ran
> rpmlint on the RPM and saw no complaints. Can you tell me how you produced
> this?
> 
> (mcpierce@mcpierce-laptop:rubygem-state_machine)$ rpmlint
> ~/Download/rubygem-state_machine-0.7.4-2.fc11.noarch.rpm 
> 1 packages and 0 specfiles checked; 0 errors, 0 warnings.

You should run rpmlint on both result rpm and src rpm. This warning is from src rpm.

> Updated spec: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine.spec
> Updated SRPM:
> http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-2.fc11.noarch.rpm

(note, this is not srpm)

> Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1446813  

[1] http://koji.fedoraproject.org/koji/getfile?taskID=1446816&name=build.log

sorry for late response
Comment 4 Darryl L. Pierce 2009-07-09 16:06:32 EDT
(In reply to comment #3)
> > > - file listed twice
> > >   warning: File listed twice:
> > > /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4/LICENSE
> 
> You can see this warning at the end of build.log [1].

I'm not sure what the issue is there. The spec file does not mention it more than once, and the file only shows up once in the gem itself. Any recommendation on how to proceed, and is this a blocker for review?

> > Updated spec: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine.spec
> > Updated SRPM:
> > http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-2.fc11.noarch.rpm
> 
> (note, this is not srpm)

Sorry, my bad, I pasted the wrong URL. The correct one is:

http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-2.fc11.src.rpm
Comment 5 Michal Ingeli 2009-07-09 16:52:42 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > > > - file listed twice
> > > >   warning: File listed twice:
> > > > /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4/LICENSE
> > 
> > You can see this warning at the end of build.log [1].
> 
> I'm not sure what the issue is there. The spec file does not mention it more
> than once, and the file only shows up once in the gem itself. Any
> recommendation on how to proceed, and is this a blocker for review?

Yes, it is a blocker. According to [1][2] files can be listend only once. It's quite easy fix. You should replace the "%{gemdir}/gems/%{gemname}-%{version}/" entry with "dir %{geminstdir}" and list all entries in that directory separately. Just note that examples, tests, rdoc and so should be listed as %doc.

[1] https://fedoraproject.org/wiki/Packaging/ReviewGuidelines#cite_ref-12
[2] https://bugzilla.redhat.com/show_bug.cgi?id=504469#c7
Comment 6 Darryl L. Pierce 2009-07-11 10:42:48 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > > > - file listed twice
> > > > >   warning: File listed twice:
> > > > > /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4/LICENSE
> > > 
> > > You can see this warning at the end of build.log [1].
> > 
> > I'm not sure what the issue is there. The spec file does not mention it more
> > than once, and the file only shows up once in the gem itself. Any
> > recommendation on how to proceed, and is this a blocker for review?
> 
> Yes, it is a blocker. According to [1][2] files can be listend only once. It's
> quite easy fix. You should replace the "%{gemdir}/gems/%{gemname}-%{version}/"
> entry with "dir %{geminstdir}" and list all entries in that directory
> separately. Just note that examples, tests, rdoc and so should be listed as
> %doc.
> 
> [1] https://fedoraproject.org/wiki/Packaging/ReviewGuidelines#cite_ref-12
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=504469#c7  

Thanks for the advice. I've fixed the aforementioned bugs in the srpm and rpm, and both check out clean for me:

Updated SPEC: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine.spec
Updated SRPM: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-3.fc11.src.rpm
Comment 7 Michal Ingeli 2009-07-12 13:19:57 EDT
Looks fine, just few more things. 
- package must own %{geminstdir}
  "file /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4 is not owned by any package"
- put examples into %doc
Comment 8 Darryl L. Pierce 2009-07-13 09:08:36 EDT
(In reply to comment #7)
> Looks fine, just few more things. 
> - package must own %{geminstdir}
>   "file /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4 is not owned by any
> package"
> - put examples into %doc  

Thank you for the advice. I've made the necessary changes, updated files are as follows:

Updated SPEC: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine.spec
Updated SRPM: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-4.fc11.src.rpm
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1471250
Comment 9 Michal Ingeli 2009-07-13 11:22:22 EDT
(In reply to comment #8)
> (In reply to comment #7)
> > Looks fine, just few more things. 
> > - package must own %{geminstdir}
> >   "file /usr/lib/ruby/gems/1.8/gems/state_machine-0.7.4 is not owned by any
> > package"

"%{geminstdir}" must be listed as "%dir %{geminstdir}", because listing directory works recursively. So now you can see a lot of twice listed files in build.log. I am approving this package, but you should fix this before import.

> > - put examples into %doc  

OK

> Thank you for the advice. I've made the necessary changes, updated files are as
> follows:
> 
> Updated SPEC: http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine.spec
> Updated SRPM:
> http://mcpierce.fedorapeople.org/rpms/rubygem-state_machine-0.7.4-4.fc11.src.rpm
> Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1471250  

One more advice, look at [1] and replace %define with %global.

APPROVED

[1] https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define
Comment 10 Darryl L. Pierce 2009-07-13 11:38:52 EDT
New Package CVS Request
=======================
Package Name: rubygem-state_machine
Short Description: Adds support for creating state machines for attributes on any
Ruby class
Owners: mcpierce
Branches: F-10 F-11 
InitialCC:
Comment 11 Kevin Fenzi 2009-07-14 00:37:49 EDT
cvs done.
Comment 12 Darryl L. Pierce 2009-07-14 08:57:10 EDT
Everything's good. Thanks, guys.

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