Bug 588437 - Review Request: rubygem-fakefs - A fake filesystem for Ruby
Review Request: rubygem-fakefs - A fake filesystem for Ruby
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: buildr
  Show dependency treegraph
 
Reported: 2010-05-03 13:57 EDT by Adam Young
Modified: 2010-12-01 13:14 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-11-18 09:51:41 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Adam Young 2010-05-03 13:57:19 EDT
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 13:40:58 EDT
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 14:15:52 EDT
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 14:24:36 EDT
(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 14:55:28 EDT
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 15:24:13 EDT
"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 13:20:24 EDT
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 12:25:38 EDT
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 15:11:03 EDT
Please post the URL of srpm, not rebuilt binary rpm.
Comment 10 Mamoru TASAKA 2010-05-12 12:21:41 EDT
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 12:49:14 EDT
One more
  - .gitignore file is not needed.
Comment 12 Mamoru TASAKA 2010-05-21 12:43:44 EDT
ping?
Comment 13 Adam Young 2010-05-21 12:48:11 EDT
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 09:51:41 EST
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.