Bug 518083 - Review Request: rubygem-pathname2 - An alternate implementation for the Pathname library.
Summary: Review Request: rubygem-pathname2 - An alternate implementation for the Pathn...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 518082
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-08-18 19:57 UTC by Brett Lentz
Modified: 2009-09-21 16:56 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-09-21 16:56:30 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Brett Lentz 2009-08-18 19:57:51 UTC
Spec URL: https://wakko.is-a-geek.com/~brett/rubygem-pathname2.spec
SRPM URL: https://wakko.is-a-geek.com/~brett/rubygem-pathname2-1.6.2-1.fc11.src.rpm
Description: An alternate implementation for the Pathname library. This version treats a path name as a String, though with certain methods overloaded.

Comment 1 Mamoru TASAKA 2009-08-20 18:16:50 UTC
Please refer to my comments on bug 518082 and uploade the new srpm
again.
(note that please change the release number of the spec file every time
 you modify your spec file to avoid confusion)

Comment 2 Brett Lentz 2009-08-20 20:13:22 UTC
New package and spec file uploaded to my webserver. Please take another look.

Comment 3 Mamoru TASAKA 2009-08-21 15:38:27 UTC
Please refer to my comments on bug 518082 again.
Additionally:
- I see %geminstdir/test is also in this spec file, so
  adding %check section is preferable.
- Please check license (see README)

Comment 4 Brett Lentz 2009-08-21 17:31:49 UTC
Ok. I've resolved the issues you mention. Please review one more time. 

https://wakko.is-a-geek.com/~brett/rubygem-pathname2.spec
https://wakko.is-a-geek.com/~brett/rubygem-pathname2-1.6.2-3.fc11.src.rpm

I've removed the %geminstdir/test directory for now. The tests require rubygem(test-unit), which isn't yet packaged.

Comment 5 Mamoru TASAKA 2009-08-21 18:56:00 UTC
(In reply to comment #4)
> I've removed the %geminstdir/test directory for now. The tests require
> rubygem(test-unit), which isn't yet packaged.  

As far as I am correct test-unit gem is needed when using ruby 1.9.x.
Now Fedora still uses ruby 1.8.x and actually test/unit.rb
exists (in ruby-libs). I guess modifying test/test_pathname.rb is
preferable to deleting this completely. Would you try this?

Comment 6 Brett Lentz 2009-08-24 20:54:41 UTC
You're right. The test_pathname.rb only needs some minor changes to run with ruby 1.8.

Fixed that, and simplified the doc dir in the %files section.

https://wakko.is-a-geek.com/~brett/rubygem-pathname2.spec
https://wakko.is-a-geek.com/~brett/rubygem-pathname2-1.6.2-4.fc11.src.rpm

Comment 7 Mamoru TASAKA 2009-08-26 19:06:53 UTC
Well,

- Please fix the following rpmlint warning:
-------------------------------------------------------------
rubygem-pathname2.noarch: W: summary-ended-with-dot An alternate implementation for the Pathname library.
-------------------------------------------------------------
  Fedora suggests that Summary should not end with dot.

! Well, maybe you like to use underscore, however you can use
  hyphen in patch name (just noting).

- Source1 should be changed to Patch0 (as this is a patch)
- By the way usually applying a patch should be done at %prep
  and use %patchX macro which rpm provides.

  Current ruby packaging guideline suggests that ruby gem file
  should be installed under %buildroot at %install directly, however
  when applying a patch or executing test is needed, I usually
  suggest to once unpack gem file under %_builddir and copy the
  unpacked tree to %buildroot at %install like:
---------------------------------------------------------------
%prep
%setup -q -c -T

%{__mkdir_p} .%{gemdir}
gem install --local --install-dir=.%{gemdir} \
   --force --rdoc %{SOURCE0}

pushd .%{geminstdir}/test
%patch0 -p0
popd

%build

%install
%{__rm} -rf %{buildroot}
%{__mkdir_p} %{buildroot}%{gemdir}
%{__cp} -a .%{gemdir}/* %{buildroot}%{gemdir}/

%check
ruby -I$(pwd)%{geminstdir}/lib $(pwd)%{geminstdir}/test/test_pathname.rb

%clean
-----------------------------------------------------------------------

Comment 9 Mamoru TASAKA 2009-08-29 15:06:47 UTC
Well, I again forgot to point out that we now recommend to use
%global instead of %define. Please fix this when importing this
to Fedora CVS.

--------------------------------------------------------------
   This package (rubygem-pathname2) is APPROVED by mtasaka
--------------------------------------------------------------

Comment 10 Brett Lentz 2009-09-02 00:51:33 UTC
Ok.  I'll change the %defines to %global before the first build.

Comment 11 Brett Lentz 2009-09-02 00:51:47 UTC
New Package CVS Request
=======================
Package Name: rubygem-pathname2
Short Description: An alternate implementation for the Pathname library
Owners: wakko666
Branches: F-12
InitialCC: mtasaka

Comment 12 Kevin Fenzi 2009-09-04 02:17:06 UTC
cvs done.

Comment 13 Mamoru TASAKA 2009-09-21 16:56:30 UTC
Closing.


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