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: nobody@fedoraproject.org ReportedBy: lkundrak@v3.sk QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com 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.
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mtasaka@ioa.s.u-tokyo.ac.jp
--- Comment #1 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 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.
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 lkundrak@v3.sk 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.
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 mtasaka@ioa.s.u-tokyo.ac.jp 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.
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 lkundrak@v3.sk 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...
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |mtasaka@ioa.s.u-tokyo.ac.jp Flag| |fedora-review?
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 mtasaka@ioa.s.u-tokyo.ac.jp 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?
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 lkundrak@v3.sk 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.
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 mtasaka@ioa.s.u-tokyo.ac.jp 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.
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(lkundrak@v3.sk)
--- Comment #8 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-06-16 12:42:16 EDT --- ping?
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 jan.klepek@hp.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |507761
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 mtasaka@ioa.s.u-tokyo.ac.jp 2009-06-26 09:21:05 EDT --- ping again?
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 lkundrak@v3.sk changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(lkundrak@v3.sk) |
--- Comment #10 from Lubomir Rintel lkundrak@v3.sk 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...
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #11 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-06-26 10:48:51 EDT --- ----------------------------------------------------------------- This package (rubygem-term-ansicolor) is APPROVED by mtasaka -----------------------------------------------------------------
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 lkundrak@v3.sk changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #12 from Lubomir Rintel lkundrak@v3.sk 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
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 tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #13 from Jason Tibbitts tibbs@math.uh.edu 2009-06-26 20:43:20 EDT --- CVS done.
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |504479
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 mtasaka@ioa.s.u-tokyo.ac.jp 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 )
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 mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
--- Comment #15 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-07-01 10:20:12 EDT --- Closing this one.
package-review@lists.fedoraproject.org