[Bug 518082] Review Request: rubygem-facade - A module that helps implement the facade pattern

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=518082


Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp




--- Comment #2 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-08-20 14:13:02 EDT ---
Some notes:

* Unneeded macros
  - %ruby_sitelib does not seem to be used.

* define -> global
  - Now Fedora recommends to use %global instead of %define for
    some reasons:
   
https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
   
https://fedoraproject.org/wiki/Packaging/Ruby#Build_Architecture_and_File_Placement

* License
  - README says facade is under "Artistic 2.0"

* About Source0
  - For gem files, please consider to use
    http://gems.rubyforge.org/gems/%{gemname}-%{version}.gem
    because with this URL you won't have to change "61520".

* Requires/BuildRequires
  - Please refer to:
    https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines
    https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Gems
    ! Some Requires are missing.

  - For BuildRequire'ing gem module, using virtual dependency is preferred
    to using the actual rpm name, ref:
    https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides
    i.e. use "BuildRequires: rubygem(rake)" instead of "BuildRequires:
rubygem-rake"

  - "Requires: ruby >= 1.8.2" is not needed (but please check the wiki
    page above)

* %setup, %build
  - Please write %setup, %build macro (even if they are empty)

* Macros usage should be consistent
  - Use macros consistently. If you want to use %mkdir_p, please use
    %rm for consistency

  - As you defines %geminstdir, please use the macro in %files

* Directory ownership issue
  - Some directories are not owned by this package properly, and
    some directories which should not be owned by this package are
    actually owned by this package. 
    Please read:
   
https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
    https://fedoraproject.org/wiki/Packaging:UnownedDirectories

    and make it sure that all directories which are newly created during
    the install of this package are correctly owned by this package.

* Test
  - As this package installs some files into under %geminstdir/test/,
    please create %check stage and execute some test programs in the stage.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.