Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: liblouis - A Braille translator and back-translator.
https://bugzilla.redhat.com/show_bug.cgi?id=597597
Summary: Review Request: liblouis - A Braille translator and back-translator. Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: lbjorn@broadpark.no QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://lamasti.net/filer/liblouis.spec SRPM URL: http://lamasti.net/filer/liblouis-1.8.0-1.fc12.src.rpm Description: Liblouis is an open-source braille translator and back-translator based on the translation routines in the BRLTTY screenreader for Linux. It has, however, gonefar beyond these routines. It is named in honor of Louis Braille. In Linux and Mac OSX it is a shared library, and in Windows it is a DLL.
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=597597
Lars Bjorndal lbjorn@broadpark.no changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: liblouis - |Review Request: liblouis - |A Braille translator and |A Braille translator and |back-translator. |back-translator. | |FE-NEEDSPONSOR
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=597597
Chen Lei supercyper1@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |supercyper1@gmail.com Blocks| |177841(FE-NEEDSPONSOR) Summary|Review Request: liblouis - |Review Request: liblouis - |A Braille translator and |A Braille translator and |back-translator. |back-translator |FE-NEEDSPONSOR |
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=597597
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.gieseking@uos.de
--- Comment #1 from Martin Gieseking martin.gieseking@uos.de 2010-06-14 11:53:56 EDT --- Hi Lars,
here are a couple of initial things that need to be fixed:
- remove the last sentence from the %description, as you're creating a binary package for Linux (so other OSes don't matter here)
- Normally, you should call make with %{?_smp_mflags}. If the package doesn't build with it, remove it and add a comment about the issue.
- .la files must not be packaged (remove them in %install and drop the corresponding line in %files)
- add at least one newline between the various sections (here: %post, %postun, %files, %changelog, ...) to increase legibility
- replace /usr/share/liblouis/* with %{_datadir}/liblouis/ and drop the %dir line
- Always try to be specific when adding something in %files in order to avoid packaging unwanted files or directories. Thus, replace %{_includedir}/* with %{_includedir}/liblouis/
- for the same reason, replace %{_mandir}/man1/* with %{_mandir}/man1/lou_*.1*
- replace %{_infodir}/liblouis.info.gz with %{_infodir}/liblouis.info* because the spec file shouldn't rely on a specific, automatically applied compression format (which could change in future rpm releases)
- add file COPYING.LIB as %doc of the base package
- The base package should only contain the library and its data files, since it's licensed under LGPLv3+. The command-line tools in %{_bindir} are licensed under GPLv3+, so they should go in a separate -tools subpackage together with file COPYING and the man1 manual pages.
- drop .f12 from the %changelog entry
That's all for now. :)
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=597597
--- Comment #2 from Lars Bjorndal lbjorn@broadpark.no 2010-06-19 15:43:33 EDT --- Thank you very much for your useful information!
The spec file found at http://lamasti.net/filer/liblouis.spec is now updated and should reflect the issues you mentioned. The release is increased, and accordingly the sources rpm is at http://lamasti.net/filer/liblouis-1.8.0-2.fc12.src.rpm.
While running rpmlint on the rpms, I get several warnings like this:
liblouis-tools.i686: W: unstripped-binary-or-object /usr/bin/lou_debug
However, building under mock, I don't get this problem. Why is it so?
Lars
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=597597
--- Comment #3 from Martin Gieseking martin.gieseking@uos.de 2010-06-20 03:05:45 EDT --- (In reply to comment #2)
While running rpmlint on the rpms, I get several warnings like this:
liblouis-tools.i686: W: unstripped-binary-or-object /usr/bin/lou_debug
I don't get these warnings. The debuginfo is only stripped if the executable flags of the binary files are set. Maybe you use an uncommon umask that disables the flags. On my system it's 0002.
Here are some more things you should address:
- rpmlint complains about the mixed use of tabs and spaces. Use either spaces or tabs for indentation, don't mix them.
- replace rm -f $RPM_BUILD_ROOT/usr/lib/liblouis.la with rm -f $RPM_BUILD_ROOT/%{_libdir}/liblouis.la otherwise your package doesn't build on 64bit archs because libraries go to /usr/lib64 there
- I suggest to also use a macro in the preceding line: rm -f $RPM_BUILD_ROOT/%{_infodir}/dir
- add AUTHORS, NEWS and ChangeLog as %doc to the base package
- documentation files should go to %{_defaultdocdir}/%{name}-%{version}/ but the .html and .txt file are placed in %{_defaultdocdir}/%{name}/. I suggest to remove the doc directory in %install and manually add the files as %doc to the base package: %doc doc/liblouis.txt doc/liblouis.html
- add file COPYING as %doc to the -tools package
- adapt the %changelog headers according to https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs + add your name + fix the release number of the latest entry + drop the dist tag .f12 Example: * Thu Jun 17 2010 Lars Bjorndal lars.bjorndal@broadpark.no 1.8.0-2
- add an empty line between the changlog entry sets to make them easier to recognize
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=597597
--- Comment #4 from Chen Lei supercyper1@gmail.com 2010-06-20 22:54:24 EDT --- (In reply to comment #2)
Thank you very much for your useful information!
The spec file found at http://lamasti.net/filer/liblouis.spec is now updated and should reflect the issues you mentioned. The release is increased, and accordingly the sources rpm is at http://lamasti.net/filer/liblouis-1.8.0-2.fc12.src.rpm.
While running rpmlint on the rpms, I get several warnings like this:
liblouis-tools.i686: W: unstripped-binary-or-object /usr/bin/lou_debug
However, building under mock, I don't get this problem. Why is it so?
Lars
Please install redhat-rpm-config/fedora-packager first.
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=597597
--- Comment #5 from Lars Bjorndal lbjorn@broadpark.no 2010-06-21 15:29:23 EDT --- In reply to Martin Gieseking martin.gieseking@uos.de
Please find an updated version of the spec file here: http://lamasti.net/filer/liblouis.spec. The new src.rpm is at: http://lamasti.net/filer/liblouis-1.8.0-3.fc12.src.rpm
You wrote: [...]
- adapt the %changelog headers according to https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs + add your name + fix the release number of the latest entry + drop the dist tag .f12 Example: * Thu Jun 17 2010 Lars Bjorndal lars.bjorndal@broadpark.no 1.8.0-2
Actually my surname is not Bjorndal, but Bjørndal. Is it ok to insert the letter ø (ø) into this field? Does it require Unicode, or is it ok to use iso8859-1?
Lars
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=597597
--- Comment #6 from Martin Gieseking martin.gieseking@uos.de 2010-06-22 01:34:28 EDT --- (In reply to comment #5)
Actually my surname is not Bjorndal, but Bjørndal. Is it ok to insert the letter ø (ø) into this field? Does it require Unicode, or is it ok to use iso8859-1?
Yes, it's of course OK to insert your name with correct spelling. In this case you should use UTF-8 encoding.
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=597597
--- Comment #7 from Martin Gieseking martin.gieseking@uos.de 2010-06-22 02:00:46 EDT --- Your spec file looks much better now. As suggested in comment #3, you can completely remove the installed documentation, e.g. with rm -rf $RPM_BUILD_ROOT/%{_defaultdocdir}/%{name}/
The files added with %doc doc/liblouis.* are directly taken from the doc directory of the uncompressed tarball.
Since I'm not a sponsor, I can't do the formal review of this package. In order to get sponsored, you usually need to provide some more packages and comment on several review requests of other packagers to show that you're familiar with the packaging guidelines.
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=597597
Till Maas opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |opensource@till.name
--- Comment #8 from Till Maas opensource@till.name 2010-06-30 10:04:27 EDT --- (In reply to comment #7)
Since I'm not a sponsor, I can't do the formal review of this package. In order to get sponsored, you usually need to provide some more packages and comment on several review requests of other packagers to show that you're familiar with the packaging guidelines.
I am a sponsor and I agree here. The additional packages are not as important as so called unofficial reviews where you comment on other package reviews.
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=597597
--- Comment #9 from Lars Bjorndal lbjorn@broadpark.no 2010-06-30 15:12:13 EDT --- In reply to comment #7 from Martin Gieseking:
Your spec file looks much better now. As suggested in comment #3, you can completely remove the installed documentation, e.g. with rm -rf $RPM_BUILD_ROOT/%{_defaultdocdir}/%{name}/
Thank you. Now, I understand.. The spec file is at: http://lamasti.net/filer/liblouis.spec. The src.rpm is at: http://lamasti.net/filer/liblouis-1.9.0-1.fc12.src.rpm.
To day, there was a new version up too, and I used that, of course.
Since I'm not a sponsor, I can't do the formal review of this package. In order to get sponsored, you usually need to provide some more packages and comment on several review requests of other packagers to show that you're familiar with the packaging guidelines.
Thank you for your helpful tips. I'll try to follow your suggestion.
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=597597
--- Comment #10 from Martin Gieseking martin.gieseking@uos.de 2010-07-05 03:28:20 EDT --- Hi Lars,
%doc doc/liblouis.* also adds the unwanted files liblouis.info and liblouis.texi. To avoid this, explicitly list the intended files, e.g. with %doc doc/liblouis.{html,txt}
Since the tarball contains a texinfo file, you could create a pdf file from it and add it as a further documentation variant. To do so, simply add "BuildRequires: texinfo-tex", call "make -C doc liblouis.pdf" in %build, and append "pdf" to the above %doc suffix list.
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=597597
--- Comment #11 from Lars Bjorndal lbjorn@broadpark.no 2010-07-06 05:29:53 EDT --- Thank you, Martin! I've followd your suggestions. The spec file is at: http://lamasti.net/filer/liblouis.spec, and the src.rpm at: http://lamasti.net/filer/liblouis-1.9.0-2.fc12.src.rpm.
I will soon follow up with a new package, liblouisxml, for another review request.
Again, thank you!
Lars
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=597597
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |615783
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=597597
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |martin.gieseking@uos.de Flag| |fedora-review?
--- Comment #12 from Martin Gieseking martin.gieseking@uos.de 2010-07-31 01:55:00 EDT --- Hi Lars,
I've recently been promoted to sponsor status, and if you're still interested in becoming a member of the packager group, I'm willing to sponsor you.
After you've been sponsored you're allowed to formally review and approve new packages submitted by other packagers. Hence, it's important that you're familiar with the packaging guidelines. To acquaint yourself with the review process and to show your (basic) understanding of the guidelines, you should do some informal reviews in the next couple of days. I'll contact you by email for further details.
Since you've already submitted liblouisxml, it's not necessary to me to provide another package. However, if you'd like to make another submission, I would do the 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=597597
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |DUPLICATE Flag|fedora-review? | Last Closed| |2011-02-16 05:33:06
--- Comment #13 from Martin Gieseking martin.gieseking@uos.de 2011-02-16 05:33:06 EST --- Closing this bug in agreement with Lars.
*** This bug has been marked as a duplicate of bug 677943 ***
package-review@lists.fedoraproject.org