[Bug 668823] Review Request: rubygem-text-format - Text::Format formats fixed-width text nicely

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=668823

--- Comment #2 from Mohammed Morsi <mmorsi@xxxxxxxxxx> 2011-02-01 18:39:14 EST ---
Thanks alot for the review. Updated the rpm based on feedback:

Spec URL: http://mo.morsi.org/files/rpms/rubygem-text-format.spec
SRPM URL:
http://mo.morsi.org/files/rpms/rubygem-text-format-1.0.0-2.fc14.src.rpm

Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2755537

If your taking this official review, please change the status of the bug to
'assigned', assign it to yourself, and change the 'fedora-review' flag to '?'



(In reply to comment #1)
> Some notes:
> 
> * License
> - Must be pick up one or set another by LICENSE file.
> 

Done

> * BuildRoot
> - On Fedora BuildRoot line is no longer needed:
> https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
> 

Done

> * %install
> - rm -rf %{buildroot} at the beginning of %install isn't needed and
> should be removed

Done

> - Gems have permission 664, need 'chmod 644'
> 

Please elaborate on this. Which files are wrong?

> * %check
> - Feel free run test suite in a %check section in the rpm
> 

Done. I decoupled this rpm from text-hyphen as it is not a runtime dependency
and incase there are issues w/ text-hyphen. Thus I only run the test suite
testing the main functionality of this gem, not the one testing it against
text-hyphen or other external components.

> * %clean
> - %clean section is no longer needed (on Fedora):
> https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean
> 

Done

> * %files
>  - Should use only the defined %geminstdir macro in %files.
> 

Done

> * documents / -doc subpackage
> - Please consider to split document files (which are not
> needed on runtime) to -doc subpackage.
> The following files/directories can be moved to -doc subpackage
> ------------------------------------------------------
> %{gemdir}/doc/%{gemname}-%{version}/
> %{geminstdir}/Changelog
> %{geminstdir}/tests/
> %{geminstdir}/Rakefile
> ------------------------------------------------------
> 

Since the original gem included these files, and there aren't too many of them,
will leave them in the main package. The files are marked as %doc though

> * rpmbuild log:
> ------------------------------------------------------
> ...
> + /usr/lib/rpm/redhat/brp-python-hardlink
> + /usr/lib/rpm/redhat/brp-java-repack-jars
> warning: File listed
> twice: /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/Changelog
> warning: File listed
> twice: /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/Install
> warning: File listed
> twice: /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/README
> + umask 022
> + cd /home/pkg/rpmbuild/BUILD
> + rm
> -rf /home/pkg/rpmbuild/BUILDROOT/rubygem-text-format-1.0.0-1.fc14.x86_64
> + exit 0
> ------------------------------------------------------
> 

Fixed

> * rpmlint log:
> ------------------------------------------------------
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/Number/%5b%5d-i.yaml
> %5b
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/Number/%5b%5d-i.yaml
> %5d
> rubygem-text-format.noarch: E:
> zero-length /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/metaconfig
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/Alpha/%5b%5d-i.yaml
> %5b
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/Alpha/%5b%5d-i.yaml
> %5d

These can be ignored.

> rubygem-text-format.noarch: W:
> hidden-file-or-dir /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/.yardoc
> rubygem-text-format.noarch: W:
> hidden-file-or-dir /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/.yardoc

Not seeing these.

> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/right_align%3f-i.yaml
> %3f
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/Roman/%5b%5d-i.yaml
> %5b
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/Roman/%5b%5d-i.yaml
> %5d
> rubygem-text-format.noarch: E:
> non-executable-script /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/Rakefile
> 0644L /usr/bin/env
> rubygem-text-format.noarch: E:
> wrong-script-end-of-line-encoding

Fixed

> /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/Rakefile
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/%3d%3d-i.yaml %3d
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/%3d%3d-i.yaml %3d
> rubygem-text-format.noarch: W:
> file-not-utf8 /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/README
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/justify%3f-i.yaml
> %3f
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/right_fill%3f-i.yaml
> %3f
> rubygem-text-format.noarch: E:
> non-executable-script
> /usr/lib/ruby/gems/1.8/gems/text-format-1.0.0/tests/testall.rb 0644L

Again not seeing some of these. Which rpmlint version are you using. I'm using
0.99, the stock version on F14.

> /usr/bin/env
> rubygem-text-format.noarch: W:
> unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/text-format-1.0.0/ri/Text/Format/left_align%3f-i.yaml
> %3f
> 2 packages and 1 specfiles checked; 4 errors, 15 warnings.
> ------------------------------------------------------

Appreciate the feedback.

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]