Bug 502490 - Review Request: rubygem-state_machine - Adds support for creating state machines for attributes on any Ruby class
Summary: Review Request: rubygem-state_machine - Adds support for creating state machi...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michal Ingeli
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-05-25 13:42 UTC by Darryl L. Pierce
Modified: 2015-06-22 00:06 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-07-14 12:57:10 UTC
Type: ---
Embargoed:
mi: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Darryl L. Pierce 2009-05-25 13:42:36 UTC
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 23:30:04 UTC
- 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 13:07:30 UTC
(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 19:47:01 UTC
(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 20:06:32 UTC
(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 20:52:42 UTC
(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 14:42:48 UTC
(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 17:19:57 UTC
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 13:08:36 UTC
(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 15:22:22 UTC
(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 15:38:52 UTC
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 04:37:49 UTC
cvs done.

Comment 12 Darryl L. Pierce 2009-07-14 12:57:10 UTC
Everything's good. Thanks, guys.


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