Re: RFS: rhinote (new upstream version)
On Sat, Feb 04, 2012 at 11:49:05AM +0100, Andrea Bolognani wrote: On Sat, Feb 04, 2012 at 12:42:57PM +0800, Paul Wise wrote: Patches look good to me, please forward them. rhinote is not in my areas of interest, so I won't be uploading it, sorry. No need to apologize, you already did so much by finding all those issues and reviewing my patches. Thank you once again for that. Any takers? David again, maybe? ;) It’s been a week now, with no response so far, so I’ll take the liberty of gently bumping the thread. The package is fairly straightforward, and the new patches have already been reviewed by a Debian Member. Please consider sponsoring it. Thank you. -- Andrea Bolognani e...@kiyuko.org Resistance is futile, you will be garbage collected. signature.asc Description: Digital signature
Re: RFS: rhinote (new upstream version)
On Sat, Feb 04, 2012 at 12:42:57PM +0800, Paul Wise wrote: Patches look good to me, please forward them. rhinote is not in my areas of interest, so I won't be uploading it, sorry. No need to apologize, you already did so much by finding all those issues and reviewing my patches. Thank you once again for that. Any takers? David again, maybe? ;) -- Andrea Bolognani e...@kiyuko.org Resistance is futile, you will be garbage collected. signature.asc Description: Digital signature
Re: RFS: rhinote (new upstream version)
On Thu, Feb 2, 2012 at 12:00 AM, Andrea Bolognani wrote: I’ve patched the source quite a bit, and I believe I’ve addressed all of your concerns. I’ve also improved integration with window managers by setting the WM_CLASS, so that pagers and the like show “Rhinote” instead of “Tk” now. ... I’ll be glad if you could review the patches to confirm they are indeed suitable for inclusion upstream, in which case I’ll forward them. And if you feel like taking a look also to the rest of the package and upload it, I certainly won’t be mad at you ;) Patches look good to me, please forward them. rhinote is not in my areas of interest, so I won't be uploading it, sorry. -- bye, pabs http://wiki.debian.org/PaulWise -- To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/CAKTje6EVooB=xsq-0ugsbto4gkhny30mdyvjj4lf6abakja...@mail.gmail.com
Re: RFS: rhinote (new upstream version)
On Tue, Jan 24, 2012 at 07:29:03PM +0800, Paul Wise wrote: I will try to conjure a patch addressing all the concerns you raised during the course of the next few days, and I hope you’ll be willing to review it when it’s done to make sure it is suitable for inclusion upstream. As I’ve mentioned earlier, I don’t have deep knowledge of Python, but you sure seem to know your way around it. Sure, no probs. I’ve patched the source quite a bit, and I believe I’ve addressed all of your concerns. I’ve also improved integration with window managers by setting the WM_CLASS, so that pagers and the like show “Rhinote” instead of “Tk” now. All of the patches are available in the git repository[1], and I’ve also uploaded an updated package to m.d.n[2]. I’ll be glad if you could review the patches to confirm they are indeed suitable for inclusion upstream, in which case I’ll forward them. And if you feel like taking a look also to the rest of the package and upload it, I certainly won’t be mad at you ;) Cheers. [1] http://anonscm.debian.org/gitweb/?p=collab-maint/rhinote.git;a=summary [2] http://mentors.debian.net/debian/pool/main/r/rhinote/rhinote_0.7.4-2.dsc -- Andrea Bolognani e...@kiyuko.org Resistance is futile, you will be garbage collected. signature.asc Description: Digital signature
Re: RFS: rhinote (new upstream version)
On Tue, Jan 24, 2012 at 08:47:21AM +0800, Paul Wise wrote:
I always get curious when people upload with no comments, so here is a review:
First of all, thank you for the review. I’m always looking forward to
improve my packages, and an in–depth review gives me the chance to do
just that.
You forgot to mention collab-maint in the changelog.
Yeah, I should probably have mentioned it, but mostly due to the fact
that I’ve added Vcs-* fields — or is collab-maint considered special
in this regard?
Do you suggest mentioning the addition in the changelog entry for the
next upload, or editing the current entry? The latter seems hacky, but
the former is a plain lie.
You didn't mention the reason for changing the priority to extra.
rhinote depends on cups-bsd, which has Priority: extra, and according
to Policy packages can’t depend on packages with lower priority.
Since I’m going to have to patch the source anyway due to the suggestions
you made below, I might try to make it use lp where available (with
fallback to lpr), and depend on cups-client | lpr (both of which are
Priority: optional) instead.
I believe I could then drop the dependency on cups-bsd | lprng, since
both packages provide lpr, and raise the priority to optional again. Is
that correct?
Please forward the desktop file and manual page upstream.
I’ve already forwarded them; upstream will probably include them in the
next upstream release, whenever that might be. Any suggestions on how to
document this fact? If we were talking about a patch, I would just use
DEP3, but this is a different scenario…
One warning from pyflakes to forward upstream:
rhinote.py:24: 'from Tkinter import *' used; unable to detect undefined names
I will notify upstream about this, maybe after doing some research to
understand what it means ;)
rhinote.py should use os.path.expanduser and instead of
os.environ['HOME'] since that works when the HOME environment variable
is not set. Please send upstream a patch for this.
Unfortunately I’m not aware of the various Python best practices due to
the fact that I only did little Python programming; thank you for pointing
this out, I’ll patch it and notify upstream.
rhinote.py should use the python subprocess module instead of the
system() function. In any case this thing will not work since it will
write to a file named lpr instead of running the lpr program. With
subprocess you can easily pipe the results of one program to another
which is what rhinote appears to want to do here. Also for upstream
(but not on Debian due to the Depends), lpr/enscript are not
guaranteed to be installed so this will fail silently instead of
informing the user that printing is not available. In addition it
doesn't remove the ~/.Rhinoteprintfile file when it is complete,
leaving files around in ~/ is rude. I would also suggest that rhinote
should be using a temporary file in the temp dir instead of a file in
~/ for this purpose, please ensure that it is secure and also supports
$TMPDIR by using the python tempfile module.
system('enscript -B --word-wrap $HOME/.Rhinoteprintfile lpr ')
Please send upstream a patch for this to use the subprocess module,
detect when enscript/lpr are available and notify the user if not, use
the tempfile module (for security and to support $TMPDIR etc) and also
clean up afterwards.
I have to admit I’m guilty of never having attempted to print using
Rhinote, mostly due to the fact that I rarely have a printer handy.
Lesson learned.
I will try to conjure a patch addressing all the concerns you raised
during the course of the next few days, and I hope you’ll be willing to
review it when it’s done to make sure it is suitable for inclusion
upstream. As I’ve mentioned earlier, I don’t have deep knowledge of
Python, but you sure seem to know your way around it.
Once again, thank you for your review.
--
Andrea Bolognani e...@kiyuko.org
Resistance is futile, you will be garbage collected.
signature.asc
Description: Digital signature
Re: RFS: rhinote (new upstream version)
On Tue, Jan 24, 2012 at 6:47 PM, Andrea Bolognani wrote: Yeah, I should probably have mentioned it, but mostly due to the fact that I’ve added Vcs-* fields — or is collab-maint considered special in this regard? Right, collab-maint isn't special here. Do you suggest mentioning the addition in the changelog entry for the next upload, or editing the current entry? The latter seems hacky, but the former is a plain lie. It isn't important to do anything, but if you want to do something, edit the current entry. rhinote depends on cups-bsd, which has Priority: extra, and according to Policy packages can’t depend on packages with lower priority. Since I’m going to have to patch the source anyway due to the suggestions you made below, I might try to make it use lp where available (with fallback to lpr), and depend on cups-client | lpr (both of which are Priority: optional) instead. I believe I could then drop the dependency on cups-bsd | lprng, since both packages provide lpr, and raise the priority to optional again. Is that correct? I'm not sure, hopefully someone else will reply to this though. It sounds like it might be right though. I’ve already forwarded them; upstream will probably include them in the next upstream release, whenever that might be. Any suggestions on how to document this fact? If we were talking about a patch, I would just use DEP3, but this is a different scenario… You can include a comment documenting the URL they have been forwarded to. For desktop files, start the comment line with # (without quotes). For the manual page, start the comment line with .\ instead. One warning from pyflakes to forward upstream: rhinote.py:24: 'from Tkinter import *' used; unable to detect undefined names I will notify upstream about this, maybe after doing some research to understand what it means ;) I think it is a deficiency in pyflakes that hints that it is a good idea to only import the things that are directly used instead of everything in the module. I have to admit I’m guilty of never having attempted to print using Rhinote, mostly due to the fact that I rarely have a printer handy. Lesson learned. Printing is something I hope I never have to do again, especially given things like this: http://nakedsecurity.sophos.com/2012/01/05/hp-patches-printer-firmware-flaw/ I will try to conjure a patch addressing all the concerns you raised during the course of the next few days, and I hope you’ll be willing to review it when it’s done to make sure it is suitable for inclusion upstream. As I’ve mentioned earlier, I don’t have deep knowledge of Python, but you sure seem to know your way around it. Sure, no probs. -- bye, pabs http://wiki.debian.org/PaulWise -- To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/caktje6gnsp8mn7j_5+eodem2hhnbog7vtohtq9vp11d4q1g...@mail.gmail.com
Re: RFS: rhinote (new upstream version)
Dear mentors, two weeks have passed without any reply, so I won’t feel bad for giving this RFS a little more visibility. The package is pretty straightforward and lintian clean, so I reckon an experienced Debian contributor could review it reasonably quickly. Please give it a go. Thank you. On Tue, Jan 10, 2012 at 10:36:12PM +0100, Andrea Bolognani wrote: Dear mentors, I am looking for a sponsor for my package rhinote. * Package name: rhinote Version : 0.7.4-1 Upstream Author : Marv Boyes greysp...@tuxfamily.org * URL : http://rhinote.tuxfamily.org/ * License : GPL-2+ Section : x11 It builds those binary packages: rhinote- virtual sticky-notes for your desktop The package is already present in the archive; this new upload would bring Debian up to speed with upstream development, along a bunch of packaging improvements. To access further information about this package, please visit the following URL: http://mentors.debian.net/package/rhinote Alternatively, one can download the package with dget using this command: dget -x http://mentors.debian.net/debian/pool/main/r/rhinote/rhinote_0.7.4-1.dsc I would be glad if someone uploaded this package for me. -- Andrea Bolognani e...@kiyuko.org Resistance is futile, you will be garbage collected. signature.asc Description: Digital signature
Re: RFS: rhinote (new upstream version)
Ciao Andrea, On Tue, 10 Jan 2012 22:36:12 +0100, Andrea Bolognani wrote: Dear mentors, I am looking for a sponsor for my package rhinote. * Package name: rhinote Version : 0.7.4-1 Upstream Author : Marv Boyes greysp...@tuxfamily.org * URL : http://rhinote.tuxfamily.org/ * License : GPL-2+ Section : x11 It builds those binary packages: rhinote- virtual sticky-notes for your desktop Uploaded, thanks for your contribution to Debian! Buona serata, David -- . ''`. Debian developer | http://wiki.debian.org/DavidPaleino : :' : Linuxer #334216 --|-- http://www.hanskalabs.net/ `. `'` GPG: 1392B174 | http://deb.li/dapal `- 2BAB C625 4E66 E7B8 450A C3E1 E6AA 9017 1392 B174 signature.asc Description: PGP signature
Re: RFS: rhinote (new upstream version)
On Mon, Jan 23, 2012 at 08:54:57PM +0100, David Paleino wrote: Uploaded, thanks for your contribution to Debian! Whoa, looks like I was right: it really was a quick job for a Debian Member ;) Thank you for the upload! -- Andrea Bolognani e...@kiyuko.org Resistance is futile, you will be garbage collected. signature.asc Description: Digital signature
Re: RFS: rhinote (new upstream version)
On Tue, Jan 24, 2012 at 5:12 AM, Andrea Bolognani wrote:
On Mon, Jan 23, 2012 at 08:54:57PM +0100, David Paleino wrote:
Uploaded, thanks for your contribution to Debian!
Whoa, looks like I was right: it really was a quick job for a
Debian Member ;)
Thank you for the upload!
I always get curious when people upload with no comments, so here is a review:
You forgot to mention collab-maint in the changelog.
You didn't mention the reason for changing the priority to extra.
Please forward the desktop file and manual page upstream.
One warning from pyflakes to forward upstream:
rhinote.py:24: 'from Tkinter import *' used; unable to detect undefined names
rhinote.py should use os.path.expanduser and instead of
os.environ['HOME'] since that works when the HOME environment variable
is not set. Please send upstream a patch for this.
rhinote.py should use the python subprocess module instead of the
system() function. In any case this thing will not work since it will
write to a file named lpr instead of running the lpr program. With
subprocess you can easily pipe the results of one program to another
which is what rhinote appears to want to do here. Also for upstream
(but not on Debian due to the Depends), lpr/enscript are not
guaranteed to be installed so this will fail silently instead of
informing the user that printing is not available. In addition it
doesn't remove the ~/.Rhinoteprintfile file when it is complete,
leaving files around in ~/ is rude. I would also suggest that rhinote
should be using a temporary file in the temp dir instead of a file in
~/ for this purpose, please ensure that it is secure and also supports
$TMPDIR by using the python tempfile module.
system('enscript -B --word-wrap $HOME/.Rhinoteprintfile lpr ')
Please send upstream a patch for this to use the subprocess module,
detect when enscript/lpr are available and notify the user if not, use
the tempfile module (for security and to support $TMPDIR etc) and also
clean up afterwards.
--
bye,
pabs
http://wiki.debian.org/PaulWise
--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive:
http://lists.debian.org/caktje6emxti3ymkbx2-joglkzfvo3r8zbqxtanrtxupuomg...@mail.gmail.com
RFS: rhinote (new upstream version)
Dear mentors, I am looking for a sponsor for my package rhinote. * Package name: rhinote Version : 0.7.4-1 Upstream Author : Marv Boyes greysp...@tuxfamily.org * URL : http://rhinote.tuxfamily.org/ * License : GPL-2+ Section : x11 It builds those binary packages: rhinote- virtual sticky-notes for your desktop The package is already present in the archive; this new upload would bring Debian up to speed with upstream development, along a bunch of packaging improvements. To access further information about this package, please visit the following URL: http://mentors.debian.net/package/rhinote Alternatively, one can download the package with dget using this command: dget -x http://mentors.debian.net/debian/pool/main/r/rhinote/rhinote_0.7.4-1.dsc I would be glad if someone uploaded this package for me. -- Andrea Bolognani e...@kiyuko.org Resistance is futile, you will be garbage collected. signature.asc Description: Digital signature
