[Bug 504469] New: Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Summary: Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

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

Summary: Review Request: rubygem-term-ansicolor - Ruby library
                    that colors strings using ANSI escape sequences
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: low
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora

SPEC: http://v3.sk/~lkundrak/gdc-ruby-stack/SPECS/rubygem-term-ansicolor.spec
SRPM:
http://v3.sk/~lkundrak/gdc-ruby-stack/SRPMS/rubygem-term-ansicolor-1.0.3-1.f...

Description:

Small Ruby library that colors strings using ANSI escape sequences.

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Mamoru Tasaka   2009-06-07 10:42:02 EDT ---
Some basic comments (please also consider to apply the following
comments to the rest of your rubygem related review requests).

- Use %global instead of %define:
  https://fedoraproject.org/wiki/Packaging/Ruby#Pure_Ruby_packages
  https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define

- Requires: ruby(abi) = 1.8 is missing
  https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines
  And for consistency I recommend to add 
  BuildRequires: ruby(abi) = 1.8

- As you have already defined %geminstdir, please use it also at
  %files and so on

- %geminstdir/[A-Z]* should be marked as %doc

- Please check if Rakefile or install.rb are needed for binary rpm.

---------------------------------------------------------------------
# Examples are documentation
mv $RPM_BUILD_ROOT%{gemdir}/gems/%{gemname}-%{version}/examples \
        $RPM_BUILD_ROOT%{gemdir}/doc/%{gemname}-%{version}
---------------------------------------------------------------------
- Note that
---------------------------------------------------------------------
$ gem contents term-ansicolor
---------------------------------------------------------------------
  expects that examples/ directory should be under %geminstdir.
  While we allow (don't forbid) to delete some files listed in 
  "$ gem contents <gemname>") if packagers think they are not needed,
  I don't think moving examples/ directory under %gemdir/doc is
  needed.

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #2 from Lubomir Rintel   2009-06-07 11:00:25 EDT ---
Thanks for picking this up

(In reply to comment #1)
> Some basic comments (please also consider to apply the following
> comments to the rest of your rubygem related review requests).
>
> Use %global instead of %define:
> https://fedoraproject.org/wiki/Packaging/Ruby#Pure_Ruby_packages
> https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define
>
> Requires: ruby(abi) = 1.8 is missing
> https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines
> And for consistency I recommend to add 
> BuildRequires: ruby(abi) = 1.8
>
> As you have already defined %geminstdir, please use it also at
> %files and so on
>
> %geminstdir/[A-Z]* should be marked as %doc

Will fix. These four (and maybe others) will be common for most other rubygem
packages I have submitted today. I'm wondering if it would make sense if I
copied this to other reviews; or what can I do to prevent duplicate reviewer
work.

Also, this was all generated by gem2rpm, we probably should fix the tool as
well.
> Please check if Rakefile or install.rb are needed for binary rpm.

Will do.
> # Examples are documentation
> mv $RPM_BUILD_ROOT%{gemdir}/gems/%{gemname}-%{version}/examples \
>         $RPM_BUILD_ROOT%{gemdir}/doc/%{gemname}-%{version}
> Note that
> $ gem contents term-ansicolor
>
> expects that examples/ directory should be under %geminstdir.
>   While we allow (don't forbid) to delete some files listed in 
>   "$ gem contents <gemname>") if packagers think they are not needed,
>   I don't think moving examples/ directory under %gemdir/doc is
>   needed.

Will revert. This applies to more packages I submitted today as well.

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #3 from Mamoru Tasaka   2009-06-07 11:06:14 EDT ---
(In reply to comment #2)
> Thanks for picking this up
>
> (In reply to comment #1)
> > Some basic comments (please also consider to apply the following
> > comments to the rest of your rubygem related review requests).

<snip>
> Will fix. These four (and maybe others) will be common for most other rubygem
> packages I have submitted today. I'm wondering if it would make sense if I
> copied this to other reviews; or what can I do to prevent duplicate reviewer
> work.

I think you can just modify your rest srpm and don't have to
copy my comments.

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #4 from Lubomir Rintel   2009-06-08 07:41:12 EDT ---
(In reply to comment #1)
> Please check if Rakefile or install.rb are needed for binary rpm.

See below.
> $ gem contents term-ansicolor
>
> expects that examples/ directory should be under %geminstdir.
>   While we allow (don't forbid) to delete some files listed in 
>   "$ gem contents <gemname>") if packagers think they are not needed,
>   I don't think moving examples/ directory under %gemdir/doc is
>   needed.

I adjusted all the packages to contain all the files "gem contents" list
(reverted the deletes). I feel that it should always be consistent with the
actual contents and don't think it's worth patching the lists for any of the
files I used to remove.

SPEC: http://v3.sk/~lkundrak/gdc-ruby-stack/SPECS/rubygem-term-ansicolor.spec
SRPM:
http://v3.sk/~lkundrak/gdc-ruby-stack/SRPMS/rubygem-term-ansicolor-1.0.3-2.f...

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #5 from Mamoru Tasaka   2009-06-08 11:50:15 EDT ---
Well, 
- Now build.log complains:
-------------------------------------------------------------
    68  warning: File listed twice:
/usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/CHANGES
    69  warning: File listed twice:
/usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/GPL
    70  warning: File listed twice:
/usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/README.en
    71  warning: File listed twice:
/usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/VERSION
--------------------------------------------------------------
  And as you did so before %geminstdir/examples should be marked as
  %doc.
  So %files should be:
--------------------------------------------------------------
%files
%defattr(-,root,root,-)

%dir %{geminstdir}
%doc %{geminstdir}/[A-Z]*
%doc %{geminstdir}/examples/
%{geminstdir}/*.rb
%{geminstdir}/lib/

%doc %{gemdir}/doc/%{gemname}-%{version}/
%{gemdir}/cache/%{gemname}-%{version}.gem
%{gemdir}/specifications/%{gemname}-%{version}.gemspec
---------------------------------------------------------------
  thoughts?

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #6 from Lubomir Rintel   2009-06-08 11:57:05 EDT ---
(In reply to comment #5)
> Well,
> Now build.log complains:
> 68  warning: File listed twice:
>
> /usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/CHANGES
>     69  warning: File listed twice:
> /usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/GPL
>     70  warning: File listed twice:
> /usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/README.en
>     71  warning: File listed twice:
> /usr/lib/ruby/gems/1.8/gems/term-ansicolor-1.0.3/VERSION

I am aware of this and believe that it is ok. Makes no real harm and saves me
from enumerating the contents of the directory just to avoid duplicate
listings.
> And as you did so before %geminstdir/examples should be marked as
>   %doc.

Will fix (for other packages as well).
> So %files should be:
>
> %files
> %defattr(-,root,root,-)
>
> %dir %{geminstdir}
> %doc %{geminstdir}/[A-Z]*
> %doc %{geminstdir}/examples/
> %{geminstdir}/*.rb
> %{geminstdir}/lib/
>
> %doc %{gemdir}/doc/%{gemname}-%{version}/
> %{gemdir}/cache/%{gemname}-%{version}.gem
> %{gemdir}/specifications/%{gemname}-%{version}.gemspec
>
> thoughts?

I can do this (and for other packages as well) if you insist on eliminate of
"listed twice" warning but I'll prefer not to.

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #7 from Mamoru Tasaka   2009-06-08 12:41:15 EDT ---
Well, I suggest to remove duplicate %files entry as
- Actually it is MUST ;)
  https://fedoraproject.org/wiki/Packaging/ReviewGuidelines#cite_ref-12

- And I am not sure if in this case rpmbuild marks these duplicated files
  as %doc or normal files.
  I actually checked the rebuilt binary rpm and it seems that
  %{geminstdir}/[A-Z]* are marked as %doc, however I don't know
  any references.

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |needinfo?([e-mail])

--- Comment #8 from Mamoru Tasaka   2009-06-16 12:42:16 EDT ---
ping?

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Jan Klepek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |507761

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #9 from Mamoru Tasaka   2009-06-26 09:21:05 EDT ---
ping again?

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Lubomir Rintel  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|needinfo?([e-mail])   |

--- Comment #10 from Lubomir Rintel   2009-06-26 09:56:37 EDT ---
Sorry, Mamoru; I've been quite busy with rest of $daywork these day (which is
quite a bad excuse for not responding). I've integrated your suggestions:

SPEC: http://v3.sk/~lkundrak/gdc-ruby-stack/SPECS/rubygem-term-ansicolor.spec
SRPM:
http://v3.sk/~lkundrak/gdc-ruby-stack/SRPMS/rubygem-term-ansicolor-1.0.3-3.f...

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #11 from Mamoru Tasaka   2009-06-26 10:48:51 EDT ---
-----------------------------------------------------------------
  This package (rubygem-term-ansicolor) is APPROVED by mtasaka
-----------------------------------------------------------------

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Lubomir Rintel  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

--- Comment #12 from Lubomir Rintel   2009-06-26 11:28:03 EDT ---
Much thanks! I'm currently in the process of fixing the duplicate file listing
in other rubygem packages as well.

New Package CVS Request
=======================
Package Name: rubygem-term-ansicolor
Short Description: Ruby library that colors strings using ANSI escape sequences
Owners: hpejakle lkundrak
Branches: F-10 F-11 EL-5

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Jason Tibbitts  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #13 from Jason Tibbitts   2009-06-26 20:43:20 EDT ---
CVS done.

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |504479

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

--- Comment #14 from Mamoru Tasaka   2009-06-27 03:06:59 EDT ---
Not applied to this package, however some notes:
- If rubygem based rpm package contains %{geminstdir}/test , would
  you try to add %check section and execute some test program like
  below?

http://cvs.fedoraproject.org/viewvc/rpms/rubygem-hpricot/devel/rubygem-hpric...
  https://fedoraproject.org/wiki/PackagingDrafts/Gem_expand_stage_change
  (The latter one is a draft and currently I am waiting from the feedback
  from any person. If you have time I will appreciate it if you
  comment on:
  https://www.redhat.com/archives/fedora-packaging/2009-June/msg00069.html
  )

[Bug 504469] Review Request: rubygem-term-ansicolor - Ruby library that colors strings using ANSI escape sequences

[Red Hat Bugzilla]

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

Mamoru Tasaka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

--- Comment #15 from Mamoru Tasaka   2009-07-01 10:20:12 EDT ---
Closing this one.