Bug 588437 - Review Request: rubygem-fakefs - A fake filesystem for Ruby
Summary: Review Request: rubygem-fakefs - A fake filesystem for Ruby
Keywords:
Status: CLOSED DEFERRED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: buildr
TreeView+ depends on / blocked
 
Reported: 2010-05-03 17:57 UTC by Adam Young
Modified: 2010-12-01 18:14 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-11-18 14:51:41 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Adam Young 2010-05-03 17:57:19 UTC
Spec URL: http://github.com/admiyo/MySpecs/blob/master/rubygem-fakefs.spec
SRPM URL: http://admiyo.fedorapeople.org/buildr-repo/rubygem-fakefs-0.2.1-1.young.src.rpm
Description: A fake filesystem.  USeful for unit testing in Ruby

Comment 1 Mamoru TASAKA 2010-05-05 17:40:58 UTC
Some notes:

* %define -> %global
  - We now prefer to use %global instead of %define.
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

* License
  - is MIT for 

* BuildRoot
  - On Fedora BuildRoot is no longer needed (rpmlint may complain about
    removing this, however you can ignore it)

* abi requirements
  - Writing "Requires: ruby(abi) = 1.8" is mandatory
    https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines

* Test
  - As this gem contains test/ directory, please add %check section and
    execute some tests there (like $ rake test)
  - Also as this gem contains spec/ directory, executing $rake spec
    is preferred.

* Duplicate files
  https://fedoraproject.org/wiki/Packaging/Guidelines#Duplicate_Files
--------------------------------------------------------------------
    57  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/fakefs-0.2.1/LICENSE
    58  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/fakefs-0.2.1/README.markdown
--------------------------------------------------------------------
  - Please make it sure that every file is listed only once in %files
    section.

* Consistent macro usage
  - As %geminstdir macro is defined, please use this macro in %files
    consistently.

Comment 2 Adam Young 2010-05-05 18:15:52 UTC
Thanks, Mamoru.  I'm assuming that most of these comments apply for the other gem2rpm packages that I have submitted.  I'll update each of those bugs  once I've corrected them.

Comment 3 Mamoru TASAKA 2010-05-05 18:24:36 UTC
(In reply to comment #1)
> * License
>   - is MIT for 

Ah... I meant "License is MIT for this package".

Comment 4 Adam Young 2010-05-05 18:55:28 UTC
Are you sure about the MIT License?  I see the commit message in the Upstream repo that it was added, but the Gem Has "GPL2 or Ruby" in it.

Comment 5 Mamoru TASAKA 2010-05-05 19:24:13 UTC
"LICENSE" text clearly shows this is under MIT (if you are saying
gem2rpm returned the license is under GPLv2 or Ruby, please don't
rely on it)

Comment 6 Mamoru TASAKA 2010-05-07 17:20:24 UTC
Just a reminder:

Every time you modify your spec file, please

- change the release number to avoid confusion
- add proper %changelog entry
- post the URLs of new srpm / spec on the review request bug
  ( please post ! )

Otherwise no one will notice you did some work after (potential)
reviewer added a comment.

Comment 7 Adam Young 2010-05-11 16:25:38 UTC
Updated with changes noted above.  Please confirm that my apporach to handling the docs is correct.

http://github.com/admiyo/MySpecs/blob/ade6668b8ca6c54dc75e0e8377b3d3d00f197854/rubygem-fakefs.spec

http://admiyo.fedorapeople.org/buildr-repo/rubygem-fakefs-0.2.1-2.young.noarch.rpm

Comment 8 Mamoru TASAKA 2010-05-11 19:11:03 UTC
Please post the URL of srpm, not rebuilt binary rpm.

Comment 10 Mamoru TASAKA 2010-05-12 16:21:41 UTC
For -2:

* Unused macro / consistent macro usage
  - %ruby_sitelib macro seems to be used no where.

  - %geminstdir should also be used also in %check.

* The place of documents
  - I don't see any reason you should move README.markdown or
    so to under %{_defaultdocdir}

* Documents
  - CONTRIBUTORS, Rakefile files should be marked as %doc.
  - spec/ test/ directories should also be marked as %doc.

Comment 11 Mamoru TASAKA 2010-05-12 16:49:14 UTC
One more
  - .gitignore file is not needed.

Comment 12 Mamoru TASAKA 2010-05-21 16:43:44 UTC
ping?

Comment 13 Adam Young 2010-05-21 16:48:11 UTC
Just one of many buildr related rubygems.  PLus, I've been diverted to other tasks.  I'll get to this, or someone from the candlepin team will, in the near future.

Comment 14 Jesus M. Rodriguez 2010-11-18 14:51:41 UTC
This initially started as a dependency for rubygem-buildr. It is no longer
needed for that. We can cancel this review.


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