Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: cups-bjnp- cups backend for Canon bjnp (USB over IP) printers
https://bugzilla.redhat.com/show_bug.cgi?id=500476
Summary: Review Request: cups-bjnp- cups backend for Canon bjnp (USB over IP) printers Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: louis@lagendijk.xs4all.nl QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
This is my first package for Fedora, so I am looking for a sponsor!
Spec URL: http://www.fazant.net/cups-bjnp/cups-bjnp.spec SRPM URL: http://www.fazant.net/cups-bjnp/cups-bjnp-0.5.2.tar.gz Description: This package contains a backend for CUPS for Canon printers using the proprietary BJNP network protocol.
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=500476
Louis Lagendijk louis@lagendijk.xs4all.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(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=500476
--- Comment #1 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-05-12 17:49:50 EDT --- I am the author and upstream maintainer of this backend. It has been available on sourceforge for some time. The package is included in FreeBSD ports collection. Similar code is also included in Sane 1.20 for support of Canon network scanners.
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=500476
--- Comment #2 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-05-16 11:16:15 EDT --- Updated version: Spec URL: http://www.fazant.net/cups-bjnp/cups-bjnp.spec SRPM URL: http://www.fazant.net/cups-bjnp/cups-bjnp-0.5.2-2.fc10.src.rpm
- removed superfluous imake dependency
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=500476
Igor Jurišković juriskovic.igor@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |juriskovic.igor@gmail.com
--- Comment #3 from Igor Jurišković juriskovic.igor@gmail.com 2009-05-18 16:17:52 EDT --- Hello Louis.
This is not official review. I'm looking for a sponsor like you. ---------------------------------------------------------------- - remove Vendor tag. Its not needed. ----------------------------------------------------------------
---------------------------------------------------------------- - Consider using parallel make if possible. If not write in comment above make why not. https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make ----------------------------------------------------------------
---------------------------------------------------------------- - install doesn't preserve timestamps. Add INSTALL="install -p" like: make DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" install http://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25install_sectio... ----------------------------------------------------------------
---------------------------------------------------------------- - update package to new version. ----------------------------------------------------------------
---------------------------------------------------------------- - I'm unable to download source but are you sure that License is only GPL? If possible be more specific like GPLv3+... ----------------------------------------------------------------
---------------------------------------------------------------- - %doc part can go into one line. Its easier to read but its your choice. ----------------------------------------------------------------
---------------------------------------------------------------- - you need to make space between changelogs:
* Thu Mar 12 2009 Louis Lagendijk llagendijk@users.sourceforge.net - something
* Thu Feb 19 2009 Louis Lagendijk llagendijk@users.sourceforge.net - something
* ...
---------------------------------------------------------------- - you're missing "-" at the beginning of some changes. ----------------------------------------------------------------
---------------------------------------------------------------- - changelog part is missing version and release number:
* Thu Mar 12 2009 Louis Lagendijk llagendijk@users.sourceforge.net <version>-<release> ----------------------------------------------------------------
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=500476
--- Comment #4 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-05-18 17:11:06 EDT --- (In reply to comment #3)
Hello Louis.
This is not official review. I'm looking for a sponsor like you.
- remove Vendor tag. Its not needed.
good catch, a left over from the Sourceforge release
- Consider using parallel make if possible. If not write in comment above make
why not. https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
done
- install doesn't preserve timestamps. Add INSTALL="install -p" like:
make DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" install http://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25install_sectio...
done
- update package to new version.
Not sure what you mean here. Anyhow, the versions available now are based on the version I released on sourceforge Sat May 16
- I'm unable to download source but are you sure that License is only GPL? If
possible be more specific like GPLv3+...
See license included. It is actually the Cups license, which is GPLv2. Corrected.
- %doc part can go into one line. Its easier to read but its your choice.
Done
- you need to make space between changelogs:
- Thu Mar 12 2009 Louis Lagendijk llagendijk@users.sourceforge.net
- something
- Thu Feb 19 2009 Louis Lagendijk llagendijk@users.sourceforge.net
- something
- ...
Done
- you're missing "-" at the beginning of some changes.
Done
- changelog part is missing version and release number:
- Thu Mar 12 2009 Louis Lagendijk llagendijk@users.sourceforge.net
<version>-<release>
Done
I found a few more issues when I ran rpmlint on the srp. These are corrected as well.
New versions available: Spec URL: http://www.fazant.net/cups-bjnp/cups-bjnp.spec SRPM URL: http://www.fazant.net/cups-bjnp/cups-bjnp-0.5.3-2.fc10.src.rpm
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=500476
--- Comment #5 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-05-18 17:43:10 EDT --- Succesfully built for F9: https://koji.fedoraproject.org/koji/taskinfo?taskID=1361803
f10: https://koji.fedoraproject.org/koji/taskinfo?taskID=1361790
F11: https://koji.fedoraproject.org/koji/taskinfo?taskID=1361810
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=500476
--- Comment #6 from Igor Jurišković juriskovic.igor@gmail.com 2009-05-20 16:41:55 EDT --- - release is 2 but in last changelog entry its 1.
- remove INSTALL from %doc. You are making installation so other users have no need to look at it.
Other than that don't see any other problems.
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=500476
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?
--- Comment #7 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-05-22 12:04:25 EDT --- For -2:
* tarball - Tarball in your srpm does not match which can be downloaded from the URL written as %SOURCE0: ------------------------------------------------------------------ 117082 2009-05-17 05:00 cups-bjnp-0.5.3.tar.gz 139933 2009-05-19 05:59 cups-bjnp-0.5.3-2.fc10.src/cups-bjnp-0.5.3.tar.gz ------------------------------------------------------------------
* Macros - Please use macros for standard directories. /usr should be either %_prefix or %_exec_prefix: https://fedoraproject.org/wiki/Packaging/RPMMacros
* %defattr - Now we recommend %defattr(-,root,root,-)
* Directory ownership issue - %_exec_prefix/lib/cups/backend (defined as %cups_backend_dir) is already owned by cups rpm and this package should not own this directory itself ! Note that ------------------------------------------------------------------ %files %{cups_backend_dir} ------------------------------------------------------------------ contains all files/directories under %cups_backend_dir and the directory %cups_backend_dir itself As this directory is owned by cups (and currently this package Requires cups-libs but not cups), add "Requires: cups" to resolve directory ownership issue.
* Documents - As Igor pointed out on the comment 6, "INSTALL" file is not likely to be needed.
* %changelog - And modify %changelog entry
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=500476
--- Comment #8 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-05-29 17:50:48 EDT --- (In reply to comment #7) Thanks for the review. I have been on a short holiday, hence the delay in responding
I believe I have now fixed all your comments:
For -2:
- tarball
- Tarball in your srpm does not match which can be downloaded from the URL written as %SOURCE0:
Ok, fixed this. Released 0.5.4 upstream and used that to make the new spec file
- Macros
- Please use macros for standard directories. /usr should be either %_prefix or %_exec_prefix: https://fedoraproject.org/wiki/Packaging/RPMMacros
Done
- %defattr
- Now we recommend %defattr(-,root,root,-)
Done
- Directory ownership issue
- %_exec_prefix/lib/cups/backend (defined as %cups_backend_dir) is already owned by cups rpm and this package should not own this directory itself ! Note that
%files %{cups_backend_dir}
contains all files/directories under %cups_backend_dir and the directory %cups_backend_dir itself As this directory is owned by cups (and currently this package Requires cups-libs but not cups), add "Requires: cups" to resolve directory ownership issue.
Thanks, completely missed this. Fixed!
- Documents
- As Igor pointed out on the comment 6, "INSTALL" file is not likely to be needed.
Done, INSTALL removed
- %changelog
- And modify %changelog entry
Done
New versions available: Spec URL: http://www.fazant.net/cups-bjnp/cups-bjnp.spec SRPM URL: http://www.fazant.net/cups-bjnp/cups-bjnp-0.5.4-1.fc10.src.rpm
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=500476
--- Comment #9 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-05-30 12:31:56 EDT --- For 0.5.4-1:
* cups_backend_dir - Well, according to cups.spec on Fedora %cups_backend_dir is %{_exec_prefix}/lib/cups, not %{_libdir}/cups/backend ( note that on 64 bit architecture, %_exec_prefix is /usr, while %_libdir is /usr/lib64 )
* %setup - "%setup -q -n %{name}-%{version}" can be replaced by "%setup -q"
Then: ------------------------------------------------------------- NOTE: Before being sponsored:
This package will be accepted with another few work. But before I accept this package, someone (I am a candidate) must sponsor you.
Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review)
When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request.
Fedora package collection review requests which are waiting for someone to review can be checked on my wiki page: http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets (Check "No one is reviewing")
Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
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=500476
--- Comment #10 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-05-30 16:46:34 EDT --- (In reply to comment #9)
For 0.5.4-1:
- cups_backend_dir
- Well, according to cups.spec on Fedora %cups_backend_dir is %{_exec_prefix}/lib/cups, not %{_libdir}/cups/backend ( note that on 64 bit architecture, %_exec_prefix is /usr, while %_libdir is /usr/lib64 )
corrected. I got misled by the description of the macros on the Wiki. Using the same as what cups does is indeed the bet way. Thanks!
- %setup
- "%setup -q -n %{name}-%{version}" can be replaced by "%setup -q"
Corrected
Then:
NOTE: Before being sponsored:
This package will be accepted with another few work. But before I accept this package, someone (I am a candidate) must sponsor you.
Thanks for the offer. I will start working on some reviews and/or other packages as soon as possible. I seem to have picked a bad time to apply as packager with first a holiday and now a business trip coming up. But first there is an updated package version available: Spec URL: http://www.fazant.net/cups-bjnp/cups-bjnp.spec SRPM URL: http://www.fazant.net/cups-bjnp/cups-bjnp-0.5.4-2.fc10.src.rpm
Thanks for your excellent input, guidance and 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=500476
--- Comment #11 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-06-03 09:23:43 EDT --- Okay, now this package itself can be approved, so I will wait for your pre-review or another review request.
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=500476
--- Comment #12 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-06-06 12:35:34 EDT --- First pre-review done. See https://bugzilla.redhat.com/show_bug.cgi?id=502693
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=500476
Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) | Summary|Review Request: cups-bjnp- |Review Request: cups-bjnp - |cups backend for Canon bjnp |cups backend for Canon bjnp |(USB over IP) printers |(USB over IP) printers Flag|fedora-review? |fedora-review+
--- Comment #13 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-06-06 15:45:14 EDT --- Okay.
-------------------------------------------------------- This package (cups-bjnp) is APPROVED by mtasaka --------------------------------------------------------
Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Install the Client Tools (Koji)".
Now I am sponsoring you.
If you want to import this package into Fedora 9/10/11, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). ! But I don't recommend to request for Fedora 9 branch because Fedora 9 will be EOF after about one month.
If you have questions, please ask me.
Removing 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=500476
--- Comment #14 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-06-06 16:08:04 EDT --- Thanks for the quick response and sponsoring me!
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=500476
--- Comment #15 from Louis Lagendijk louis@lagendijk.xs4all.nl 2009-06-11 13:47:25 EDT --- New Package CVS Request ======================= Package Name: cups-bjnp Short Description: CUPS backend for the Canon BJNP network printers Owners: llagendijk Branches: F-10 F-11 EL-5 InitialCC:
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=500476
Louis Lagendijk louis@lagendijk.xs4all.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
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=500476
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #16 from Kevin Fenzi kevin@tummy.com 2009-06-12 00:13:50 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=500476
--- Comment #17 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-06-14 12:28:49 EDT --- Now closing.
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=500476
Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
package-review@lists.fedoraproject.org