[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-10-21 Thread 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=458866





--- Comment #28 from Fedora Update System upda...@fedoraproject.org  
2009-10-21 12:24:49 EDT ---
xls2csv-1.06-5.el5 has been pushed to the Fedora EPEL 5 stable repository.  If
problems still persist, please make note of it in this bug report.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-10-21 Thread 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=458866


Fedora Update System upda...@fedoraproject.org changed:

   What|Removed |Added

   Flag|needinfo?(pa...@hubbitus.in |
   |fo) |
 Status|ON_QA   |CLOSED
   Fixed In Version||1.06-5.el5
 Resolution||ERRATA




-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-10-20 Thread 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=458866





--- Comment #26 from Fedora Update System upda...@fedoraproject.org  
2009-10-20 20:38:47 EDT ---
xls2csv-1.06-5.fc10 has been pushed to the Fedora 10 stable repository.  If
problems still persist, please make note of it in this bug report.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-10-20 Thread 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=458866





--- Comment #27 from Fedora Update System upda...@fedoraproject.org  
2009-10-20 20:56:48 EDT ---
xls2csv-1.06-5.fc11 has been pushed to the Fedora 11 stable repository.  If
problems still persist, please make note of it in this bug report.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-25 Thread 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=458866





--- Comment #22 from Fedora Update System upda...@fedoraproject.org  
2009-09-25 04:13:59 EDT ---
xls2csv-1.06-5.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/xls2csv-1.06-5.fc10

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-25 Thread 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=458866





--- Comment #23 from Fedora Update System upda...@fedoraproject.org  
2009-09-25 04:24:34 EDT ---
xls2csv-1.06-5.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/xls2csv-1.06-5.fc11

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-25 Thread 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=458866





--- Comment #24 from Fedora Update System upda...@fedoraproject.org  
2009-09-25 04:43:04 EDT ---
xls2csv-1.06-5.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/xls2csv-1.06-5.el5

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-25 Thread 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=458866


Fedora Update System upda...@fedoraproject.org changed:

   What|Removed |Added

 Status|ASSIGNED|ON_QA




--- Comment #25 from Fedora Update System upda...@fedoraproject.org  
2009-09-25 21:27:44 EDT ---
xls2csv-1.06-5.el5 has been pushed to the Fedora EPEL 5 testing repository.  If
problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update xls2csv'.  You can provide
feedback for this update here:
http://admin.fedoraproject.org/updates/EL-5/FEDORA-EPEL-2009-0514

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-24 Thread 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=458866


Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info changed:

   What|Removed |Added

   Flag|needinfo?(pa...@hubbitus.in |
   |fo) |




--- Comment #17 from Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info  
2009-09-24 03:55:45 EDT ---
You are shure?
I try check it:
# repoquery --whatprovides /bin/xls2csv
but nothing found...

In what branches you found this conflict?

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-24 Thread 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=458866





--- Comment #18 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp  2009-09-24 
04:09:16 EDT ---
Not '/bin/xls2csv' but actually '/usr/bin/xls2csv'.
catdoc (in Fedora) includes this file.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-24 Thread 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=458866





--- Comment #19 from Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info  
2009-09-24 04:34:20 EDT ---
Oh, yes /usr/bin/xls2csv there...

So, what you think how I should name it?

xlsTOcsv
convertxls2csv
convertXls2csv
convertXls2Csv
something other?

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-24 Thread 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=458866


Jan Klepek jan.kle...@hp.com changed:

   What|Removed |Added

   Flag||needinfo?(pa...@hubbitus.in
   ||fo)




--- Comment #20 from Jan Klepek jan.kle...@hp.com  2009-09-24 05:16:16 EDT ---
convertxls2csv sounds fine.
please take in mind that you need to rename man pages too

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-24 Thread 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=458866





--- Comment #21 from Mamoru Tasaka mtas...@ioa.s.u-tokyo.ac.jp  2009-09-24 
05:21:18 EDT ---
I don't know this package well so for renaming I will leave
it to you (convertxls2csv seems fine also for me)

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-23 Thread 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=458866


Jan Klepek jan.kle...@hp.com changed:

   What|Removed |Added

   Flag||needinfo?(pa...@hubbitus.in
   ||fo)




--- Comment #16 from Jan Klepek jan.kle...@hp.com  2009-09-23 16:43:12 EDT ---
I have just found that there is another package which is in fedora for long
time which owns /bin/xls2csv. Therefore could you please rename your
binary/manpage to something else during %install.

Otherwise it would lead to conflict when somebody installs catdoc package and
after that they want to install xls2csv.
http://fedoraproject.org/wiki/Packaging:Conflicts

I'm sorry that I forgot to check for this during review.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-21 Thread 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=458866





--- Comment #14 from Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info  
2009-09-21 14:48:20 EDT ---
Hi, Jan Klepek, sorry for delay

New Package CVS Request
===
Package Name: xls2csv
Short Description: A script that recodes a spreadsheet's charset and saves as
CSV
Owners: hubbitus
Branches: F-10 F-11 EL-5
InitialCC:

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-21 Thread 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=458866


Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info changed:

   What|Removed |Added

   Flag|needinfo?(pa...@hubbitus.in |fedora-cvs?
   |fo) |




--- Comment #13 from Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info  
2009-09-21 14:47:40 EDT ---
Hi, Jan Klepek, sorry for delay

New Package CVS Request
===
Package Name: 
Short Description: 
Owners: hubbitus
Branches: F-10 F-11 EL-5
InitialCC:

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-21 Thread 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=458866


Jason Tibbitts ti...@math.uh.edu changed:

   What|Removed |Added

   Flag|fedora-cvs? |fedora-cvs+




--- Comment #15 from Jason Tibbitts ti...@math.uh.edu  2009-09-21 22:07:51 
EDT ---
CVS done.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-20 Thread 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=458866


Jan Klepek jan.kle...@hp.com changed:

   What|Removed |Added

   Flag||needinfo?(pa...@hubbitus.in
   ||fo)




--- Comment #12 from Jan Klepek jan.kle...@hp.com  2009-09-20 07:05:27 EDT ---
Hi Pavel,

Please request CVS and submit build into bodhi.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-11 Thread 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=458866


Jan Klepek jan.kle...@hp.com changed:

   What|Removed |Added

   Flag|fedora-review?  |fedora-review+




--- Comment #11 from Jan Klepek jan.kle...@hp.com  2009-09-11 07:14:53 EDT ---
rpmlint must be run on every package. The output should be posted in the
review.
- OK, clean

The package must be named according to the Package Naming Guidelines.
- OK

The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption.
- ok

The package must meet the Packaging Guidelines  perl specific guidelines.
- ok

The package must be licensed with a Fedora approved license and meet the
Licensing Guidelines.
- OK

The License field in the package spec file must match the actual license.
- OK

If (and only if) the source package includes the text of the license(s) in its
own file, then that file, containing the text of the license(s) for the package
must be included in %doc.
- OK

The spec file must be written in American English.
- OK

The spec file for the package MUST be legible.
- OK

The sources used to build the package must match the upstream source, as
provided in the spec URL.
- OK

The package MUST successfully compile and build into binary rpms on at least
one primary architecture.
- OK

All build dependencies must be listed in BuildRequires.
- OK

The spec file MUST handle locales properly.
- OK

Ldconfig in %post and %postun.
- N/A

Relocatable package and /usr prefix.
- N/A

A package must own all directories that it creates.
- OK

A Fedora package must not list a file more than once in the spec file's %files
listings.
- OK

Permissions on files must be set properly.
- OK

Each package must have a correct %clean section.
- OK

Each package must consistently use macros.
- OK

The package must contain code, or permissable content.
-OK

Large documentation files must go in a -doc subpackage.
-OK

If a package includes something as %doc, it must not affect the runtime of the
application.
-OK

Header files must be in a -devel package.
-OK

Static libraries must be in a -static package.
-OK

Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' .
- N/A

Library with .so suffix must be in -devel package.
- N/A

In the vast majority of cases, devel packages must require the base package
using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
- N/A

Packages must NOT contain any .la libtool archives, these must be removed in
the spec if they are built.
- N/A

Gui application and desktop-file-install.
- N/A

Packages must not own files or directories already owned by other packages.
- OK

At the beginning of %install, each package MUST run rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
- OK

All filenames in rpm packages must be valid UTF-8.
- OK

-koji build: ok, http://koji.fedoraproject.org/koji/taskinfo?taskID=1670457
-application testing: working as expected

conclusion: Approved

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-09-10 Thread 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=458866


Jan Klepek jan.kle...@hp.com changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 CC||jan.kle...@hp.com
 AssignedTo|nob...@fedoraproject.org|jan.kle...@hp.com
   Flag||fedora-review?




-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-08-10 Thread 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=458866





--- Comment #10 from Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info  
2009-08-10 14:32:28 EDT ---
Ok, you are agai right.
Readme says: This script is free software; you can redistribute it and/or
modify it under the same terms as Perl itself., so, according to guidelines:
http://fedoraproject.org/wiki/Packaging:Perl#License_tag I change License to
GPL+ or Artistic

http://hubbitus.net.ru/rpm/Fedora11/xls2csv/xls2csv-1.06-4.fc11.src.rpm

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-08-09 Thread 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=458866





--- Comment #7 from Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info  
2009-08-09 09:43:16 EDT ---
(In reply to comment #6)
 Therefore I think writing make is just as future-proof as
 %{__make}, and it's easier to read. The macros aren't forbidden though, as
 far as I know.
I thought about it.

  But rpmbuild don't add next requires:
  perl(:MODULE_COMPAT_5.10.0)
 
 You should keep that one, according to the guidelines.
Ok.

  perl-Unicode-Map or perl(Unicode::Map)
 perl-Spreadsheet-ParseExcel depends on perl(Unicode::Map), so you get that
 pulled in by the automatic dependency on perl(Spreadsheet::ParseExcel)
Hm... Such dependency may be broken in the future. Imagine:
perl(Spreadsheet::ParseExcel) in the future may change implementation to do not
use perl(Unicode::Map). Therefore, it is not mean what this package not use it
also. So, I think it Requires: perl(Unicode::Map) should be explicit there.

 When I build the package I get an automatic dependency on 
 perl(Locale::Recode),
 which perl-libintl provides.
Ok, I wasn't saw that.

  Sorry, I don't want change tabs to spaces.
 Well, if this is what you want others to see when they read your spec, it's
 your choice:
 http://www.rombobjörn.se/diverse/xls2csv.png
 It is a requirement that the spec be legible, but this misalignment isn't bad
 enough to make it illegible, so I suppose it's allowed.  
Please, see explanations of Ralf Corsepius there -
https://bugzilla.redhat.com/show_bug.cgi?id=510865#c23 . In any case I can't
say more. Please keep in mind - I am very grateful that he came, but I did not
ask him about it myself.

BTW, Björn Persson, thank you for the help. Half way done, don't you want
review this package?

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-08-09 Thread 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=458866





--- Comment #9 from Björn Persson bj...@xn--rombobjrn-67a.se  2009-08-09 
21:04:18 EDT ---
(In reply to comment #7)
  perl-Spreadsheet-ParseExcel depends on perl(Unicode::Map), so you get that
  pulled in by the automatic dependency on perl(Spreadsheet::ParseExcel)
 Hm... Such dependency may be broken in the future. Imagine:
 perl(Spreadsheet::ParseExcel) in the future may change implementation to do 
 not
 use perl(Unicode::Map). Therefore, it is not mean what this package not use it
 also. So, I think it Requires: perl(Unicode::Map) should be explicit there.

The question is, does xls2csv use Unicode::Map directly, or does it need
Unicode::Map only because Spreadsheet::ParseExcel uses it? Unicode::Map isn't
mentioned anywhere in the code, only in the documentation. I also searched for
the names of the documented methods of Unicode::Map, and didn't find any of
them. Therefore I think that xls2csv doesn't use Unicode::Map directly, and
that if Spreadsheet::ParseExcel gets changed to not use Unicode::Map, then
xls2csv won't need it either. I'm not a Perl expert however, so I may have
missed something. You may want to ask for advice on Fedora-perl-devel-list.

 BTW, Björn Persson, thank you for the help. Half way done, don't you want
 review this package?  

I'm not qualified to do reviews. Comment and discuss like this is all I can do
until I find a sponsor, but I think this package is close to being ready for
approval.

There is one thing I haven't mentioned before because I wasn't sure what's
right, namely the license field. Using and in the license field means that
the package contains some files with one license and some other files with
another license. That's not really possible when the whole program is only one
file. The licensing guidelines also say that in such cases there must be a
comment explaining what parts are covered by which license.
(https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios)

So should it be GPL+ or Artistic or GPLv2+ or Artistic? I'm not sure. The
Perl packaging document seems to say that the same terms as Perl itself
should be translated to GPL+ or Artistic
(http://fedoraproject.org/wiki/Packaging:Perl#License_tag), but the points it
makes aren't about the distinction between GPL+ and GPLv2+. Perhaps you should
ask on Fedora-perl-devel-list about this too.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-08-07 Thread 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=458866





--- Comment #6 from Björn Persson bj...@xn--rombobjrn-67a.se  2009-08-07 
09:40:14 EDT ---
(In reply to comment #5)
  Why do you use these macros by the way? What value do they add?
 Really now - nothing. But if, when pathes changed - only one system macros
 definition must be changed too opposite direct search/replace hundreds of
 values.

You mean if the commands were to be moved to some directory not in the default
PATH?

If you were to write /usr/bin/make, and then make were moved to /bin, then
your script would break, but if you write just make, then it will still work
as long as make is in one of the directories listed in PATH. Moving these
commands out of the default PATH would annoy lots of people very very much, so
it won't happen. Therefore I think writing make is just as future-proof as
%{__make}, and it's easier to read. The macros aren't forbidden though, as
far as I know.

 But ppmbuild don't add next requires:
 perl(:MODULE_COMPAT_5.10.0)

You should keep that one, according to the guidelines.

 perl-Unicode-Map or perl(Unicode::Map)

perl-Spreadsheet-ParseExcel depends on perl(Unicode::Map), so you get that
pulled in by the automatic dependency on perl(Spreadsheet::ParseExcel)

 perl-libintl

When I build the package I get an automatic dependency on perl(Locale::Recode),
which perl-libintl provides.

 Sorry, I don't want change tabs to spaces.

Well, if this is what you want others to see when they read your spec, it's
your choice:
http://www.rombobjörn.se/diverse/xls2csv.png
It is a requirement that the spec be legible, but this misalignment isn't bad
enough to make it illegible, so I suppose it's allowed.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-08-06 Thread 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=458866


Björn Persson bj...@xn--rombobjrn-67a.se changed:

   What|Removed |Added

 CC||bj...@xn--rombobjrn-67a.se




--- Comment #4 from Björn Persson bj...@xn--rombobjrn-67a.se  2009-08-06 
17:42:38 EDT ---
· I don't see the point in first including %{buildroot} in both PREFIX and
DESTDIR and then hacking around the doubled buildroot that this causes. As far
as I can see the build works just as well without %{buildroot} in PREFIX. It
eliminates the need for the hack and makes RPMlint happy.

· The guidelines say you shouldn't use both %{buildroot} and $RPM_BUILD_ROOT.
Choose one or the other and stick to it.
(https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS)

· You changed rm to %{__rm}, so to be consistent you should use
%{__chmod} as well. (There doesn't seem to be a corresponding macro for
find.) Why do you use these macros by the way? What value do they add?

· According to the Perl packaging guidelines you shouldn't require Perl
packages directly by name, but instead require the Perl modules by the
perl(Foo) naming scheme.
(https://fedoraproject.org/wiki/Packaging:Perl#Perl_Requires_and_Provides)

· Most of the explicit dependencies seem unnecessary, because RPMbuild will add
dependencies automatically. (Requires: that is, not BuildRequires:)

· Please replace the tabs in the spec with spaces. They make the file look ugly
in my editor, and in my web browser too. Apparently your editor has a different
idea of tab width than mine.

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2009-08-06 Thread 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=458866





--- Comment #5 from Pavel Alexeev (aka Pahan-Hubbitus) pa...@hubbitus.info  
2009-08-06 19:33:51 EDT ---
Björn Persson, thank you very much for the notes.

(In reply to comment #4)
 · I don't see the point in first including %{buildroot} in both PREFIX and
 DESTDIR and then hacking around the doubled buildroot that this causes. As far
 as I can see the build works just as well without %{buildroot} in PREFIX. It
 eliminates the need for the hack and makes RPMlint happy.
I fix it.

 · The guidelines say you shouldn't use both %{buildroot} and $RPM_BUILD_ROOT.
 Choose one or the other and stick to it.
 (https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS)
Ok.

 · You changed rm to %{__rm}, so to be consistent you should use
 %{__chmod} as well.
Ok.

 (There doesn't seem to be a corresponding macro for
 find.)
Yes. I also found it strange.

 Why do you use these macros by the way? What value do they add?
Really now - nothing. But if, when pathes changed - only one system macros
definition must be changed too opposite direct search/replace hundreds of
values.

 · According to the Perl packaging guidelines you shouldn't require Perl
 packages directly by name, but instead require the Perl modules by the
 perl(Foo) naming scheme.
 (https://fedoraproject.org/wiki/Packaging:Perl#Perl_Requires_and_Provides)
 
 · Most of the explicit dependencies seem unnecessary, because RPMbuild will 
 add
 dependencies automatically. (Requires: that is, not BuildRequires:)

I remove perl and prel-modules requires.
But ppmbuild don't add next requires:
perl(:MODULE_COMPAT_5.10.0)
perl-Unicode-Map or perl(Unicode::Map)
perl-libintl
as you think, its really don't needed (excuse me, I'm dont very similar with
perl)?

Please replace the tabs in the spec with spaces. They make the file look ugly
 in my editor, and in my web browser too. Apparently your editor has a
 different idea of tab width than mine.
Yes, I think 5 spaces tab width (it is convenient to easy distinguish pace and
tabs indent in each editor). I think it is not issue. Sorry, I don't want
change tabs to spaces.


http://hubbitus.net.ru/rpm/Fedora11/xls2csv/xls2csv.spec
http://hubbitus.net.ru/rpm/Fedora11/xls2csv/xls2csv-1.06-2.fc11.src.rpm

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2008-11-22 Thread 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=458866





--- Comment #3 from Pavel Alexeev [EMAIL PROTECTED]  2008-11-22 11:25:53 EDT 
---
(In reply to comment #2)
 A couple of comments:
 
 If you're going to, for whatever reason, use macros like %__make and %{__mv},
 you need to be consistent:  either use the brackets or don't, and then you 
 need
 to use %{__rm} instead of just rm.  Or just drop the macros entirely and 
 save
 the typing.
I do not see this so much sense to pay attention. But ok, I fix it.

 rpmlint complains about the .packlist file.  Honestly I'm not sure why you
 would even need to package it, and if you check the perl package template
 you'll see that it deletes .packlist files and then goes through and deletes
 empty directory trees.  In fact, perhaps you might want to take a look at the
 template; it might give you some hints for better ways to do some things. 
 Install the rpmdevtools package and look at
 /etc/rpmdevtools/spectemplate-perl.spec.
I have long thought about it file...
Very thanks for this hints. I get few lines fronm this spec template (see spec
changelog for more details).

 In any case, it's always useful to run rpmlint on your packages (both the
 source RPM and the final built packages).
Off course!
Now it is produce only 1 warning, but it is too erroneously:

$ rpmlint -i xls2csv-1.06-1.fc9.src.rpm 
xls2csv.src:32: W: rpm-buildroot-usage %build %{__perl} Makefile.PL
INSTALLDIRS=vendor PREFIX=%{buildroot}%{_prefix}
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it will
break short circuiting.

so, it is wrong, because usage of %{buildroot} there is not touch this dir!!!
This is needed only for configuration build.

http://hubbitus.net.ru/rpm/Fedora9/xls2csv/xls2csv-1.06-1.fc9.src.rpm

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2008-11-20 Thread 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=458866





--- Comment #2 from Jason Tibbitts [EMAIL PROTECTED]  2008-11-20 22:54:25 EDT 
---
A couple of comments:

If you're going to, for whatever reason, use macros like %__make and %{__mv},
you need to be consistent:  either use the brackets or don't, and then you need
to use %{__rm} instead of just rm.  Or just drop the macros entirely and save
the typing.

rpmlint complains about the .packlist file.  Honestly I'm not sure why you
would even need to package it, and if you check the perl package template
you'll see that it deletes .packlist files and then goes through and deletes
empty directory trees.  In fact, perhaps you might want to take a look at the
template; it might give you some hints for better ways to do some things. 
Install the rpmdevtools package and look at
/etc/rpmdevtools/spectemplate-perl.spec.

In any case, it's always useful to run rpmlint on your packages (both the
source RPM and the final built packages).

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2008-08-25 Thread 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=458866


Mamoru Tasaka [EMAIL PROTECTED] changed:

   What|Removed |Added

 Blocks|177841  |




--- Comment #1 from Mamoru Tasaka [EMAIL PROTECTED]  2008-08-25 03:49:08 EDT 
---
(Removing NEEDSPONSOR: bug 455067)

-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review


[Bug 458866] Review Request: xls2csv - A script that recodes a spreadsheet's charset and saves as CSV

2008-08-16 Thread 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=458866


Jason Tibbitts [EMAIL PROTECTED] changed:

   What|Removed |Added

 Blocks||177841




-- 
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.

___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review