Bug#829151: RFS: setcolortemperature/1.1-1 ITP
> On Jul 12, 2016, at 8:08 AM, Gianfranco Costamagna > wrote: > > sponsoring soon! > > thanks for your contribution to Debian! Thank you for sponsoring my package Gianfranco!
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
Hi, > >Aren't there different rationales for source and binary package names? >This is what I have been thinking: > >- source package name should be upstream's name, because it's the > *source* >- binary package name is whatever makes most sense for someone typing > apt-get Indeed, I didn't look carefully at the package name while installing :) sponsoring soon! thanks for your contribution to Debian! G.
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
On Mon, Jul 11, 2016 at 05:13:22PM +, Gianfranco Costamagna wrote: > src:setcolortemperature > binary: sct > > please choose one, and use the same, you can't usually expect users to install > a package and expect a binary called in another way. > > I seem to be pedantic, but I really like to have them called in the same way Aren't there different rationales for source and binary package names? This is what I have been thinking: - source package name should be upstream's name, because it's the *source* - binary package name is whatever makes most sense for someone typing apt-get -- Sean Whitton
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
Hi, >K == Kelvin, not kilo. this is what happens when one person is not using google :) thanks you all! G.
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
* Gianfranco Costamagna , 2016-07-11, 17:13: are you sure it is 6500K? 6500K should be equal to "650" K == Kelvin, not kilo. -- Jakub Wilk
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
On 11 Jul 2016 19:18, "Gianfranco Costamagna" wrote: > > Hi, > > >This has been fixed. Now when -h is passed usage is printed and if the > > >temperature passed is wrong usage will also be printed. > > > I still see nothing when called with no parameters :) > > (well you might print something like "temperature reset to the default value (65K) or similar) > > BTW > If no arguments are passed sct resets the display to the default temperature (6500K) > > > are you sure it is 6500K? > > 6500K should be equal to "650" > (note: I didn't check, I just want to avoid a bad number here) K here is kelvins, not thousands :) -- Cheers, Andrew
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
Hi, >This has been fixed. Now when -h is passed usage is printed and if the >temperature passed is wrong usage will also be printed. I still see nothing when called with no parameters :) (well you might print something like "temperature reset to the default value (65K) or similar) BTW If no arguments are passed sct resets the display to the default temperature (6500K) are you sure it is 6500K? 6500K should be equal to "650" (note: I didn't check, I just want to avoid a bad number here) >Fixed. >Added nice! >> other stuff LGTM sorry but I missed something: src:setcolortemperature binary: sct please choose one, and use the same, you can't usually expect users to install a package and expect a binary called in another way. I seem to be pedantic, but I really like to have them called in the same way thanks for caring and understanding I hope :) Gianfranco
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
control: retitle -1 RFS: setcolortemperature/1.3-1 ITP On 07/08/2016 12:29 PM, Gianfranco Costamagna wrote: > > the package is quite simple, but I would appreciate something more > verbose when calling it with wrong parameters. > e.g. > sct > sct -h > sct -v > sct 10 > sudo sct 10 > sudo sct 14 > > all gives no output. > > After reading the manpage I discovered that numbers should be within a range. > > I would appreciate a little help, and some error messages when bad input is > provided. This has been fixed. Now when -h is passed usage is printed and if the temperature passed is wrong usage will also be printed. > other issues: > $(CC) sct.c $(CFLAGS) $(LDFLAGS) -Wall -lX11 -lXrandr -o sct > > > missing CPPFLAGS > > LDFLAGS should go at the bottom, to avoid link failures with wl,asneeded > (e.g. on Ubuntu where it is the default) Fixed. > there is a missing license in the tarball, please ask upstream to provide one Added. > other stuff LGTM > > G. > -- Jacob Adams GPG Key: AF6B 1C26 E2D0 A988 432B 94F4 24C0 2B85 B59F E5A9
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
control: owner -1 ! control: tags -1 moreinfo >Thanks for all your help reviewing my package! the package is quite simple, but I would appreciate something more verbose when calling it with wrong parameters. e.g. sct sct -h sct -v sct 10 sudo sct 10 sudo sct 14 all gives no output. After reading the manpage I discovered that numbers should be within a range. I would appreciate a little help, and some error messages when bad input is provided. other issues: $(CC) sct.c $(CFLAGS) $(LDFLAGS) -Wall -lX11 -lXrandr -o sct missing CPPFLAGS LDFLAGS should go at the bottom, to avoid link failures with wl,asneeded (e.g. on Ubuntu where it is the default) there is a missing license in the tarball, please ask upstream to provide one other stuff LGTM G.
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
control: tag -1 -moreinfo On 07/07/2016 10:38 PM, Sean Whitton wrote: > You didn't bump it in the Debian changelog :) > > I consider this package ready to upload to Debian (packaging repo commit > 6f964da0, main repo commit 00b97fee), except for: > > - fix version in the Debian changelog > - re-run `dch -r' to refresh the changelog timestamp > > When you've done those, please remove the moreinfo tag from this bug. I fixed up the changelog. Thanks for all your help reviewing my package! -- Jacob Adams GPG Key: AF6B 1C26 E2D0 A988 432B 94F4 24C0 2B85 B59F E5A9 signature.asc Description: OpenPGP digital signature
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
control: tag -1 +confirmed control: noowner -1 Hello, On Thu, Jul 07, 2016 at 10:21:40PM -0400, Jacob Adams wrote: > Should be fixed now. > I just signed the 1.1 tarball. > I'm not sure why uscan was trying to fetch that anyway as I bumped the > version number to 1.2 after my last changes. You didn't bump it in the Debian changelog :) I consider this package ready to upload to Debian (packaging repo commit 6f964da0, main repo commit 00b97fee), except for: - fix version in the Debian changelog - re-run `dch -r' to refresh the changelog timestamp When you've done those, please remove the moreinfo tag from this bug. Thanks! -- Sean Whitton
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
On 07/07/2016 10:13 PM, Sean Whitton wrote: > Hello, > > Unfortunately your watch file doesn't seem to work now. > > uscan warn: In directory ., downloading > > https://github.com/Tookmund/setcolortemperature/releases/download/v1.1/setcolortemperature-1.1.tar.gz.asc > failed: 404 Not Found > Should be fixed now. I just signed the 1.1 tarball. I'm not sure why uscan was trying to fetch that anyway as I bumped the version number to 1.2 after my last changes. -- Jacob Adams GPG Key: AF6B 1C26 E2D0 A988 432B 94F4 24C0 2B85 B59F E5A9 signature.asc Description: OpenPGP digital signature
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
Hello, Unfortunately your watch file doesn't seem to work now. uscan warn: In directory ., downloading https://github.com/Tookmund/setcolortemperature/releases/download/v1.1/setcolortemperature-1.1.tar.gz.asc failed: 404 Not Found -- Sean Whitton
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
control: retitle -1 RFS: setcolortemperature/1.2-1 ITP On 07/07/2016 02:00 AM, Sean Whitton wrote: > Hello, > > Since you are upstream, would you consider providing a changelog that > you can install? Lintian is saying no-upstream-changelog and it seems > we can easily fix that :) > I added a changelog. While I was at it I fixed the compiler warnings and signed the release. 100% lintian clean now :) -- Jacob Adams GPG Key: AF6B 1C26 E2D0 A988 432B 94F4 24C0 2B85 B59F E5A9 signature.asc Description: OpenPGP digital signature
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
Hello, Since you are upstream, would you consider providing a changelog that you can install? Lintian is saying no-upstream-changelog and it seems we can easily fix that :) -- Sean Whitton
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
On 07/05/2016 08:02 AM, Sean Whitton wrote: > control: owner -1 ! > control: tag -1 +moreinfo > > Dear Jacob, > > This looks like a nice alternative to redshift-gtk. Thanks for > packaging it. I can't sponsor the upload, but I hope this review is > useful to you. Thanks! The review definitely helped. > > 1. Could you explain why you are packaging your fork rather then the >original? (This kind of thing should go in the ITP.) The original was released in a blog post as a sort of code dump (http://www.tedunangst.com/flak/post/sct-set-color-temperature ) I thought it would improve long term maintainability if there was a real build system and active upstream. It's pretty hard to package software without a tarball or VCS. If this is discouraged, I'd be happy to email the real upstream about it, but I simply thought this would be easier for everyone. If I need to send a mail explaining this to the ITP I can do that. > > 2. Could you put your Debian packaging in git, please? It makes >reviewing easier. Perhaps as a 'debian' branch of your repo. Whoops. Actually I already do this in a different git repo and I just filled out the relevant fields of d/control incorrectly. Thanks for catching that! > > 3. The formatting of the long description in debian/control is a bit >strange. Please separate paragraphs using a line with the string >" .". Probably best to wrap at 70 chars, too. I think I fixed this now. > > 4. The wording of the long description could be improved. The first >sentence isn't really a sentence -- it would be better to write "sct >is a small C program to change the screen color temperature. It can >be used to reduce or increase the amount of blue light produced by >the screen." Please take another look at your wording :) Wow that description was a mess. I've fixed it up now and I think it's much clearer. > 5. Have you considered calling the binary package 'sct'? That is what >someone might guess when they want to install this with apt-get. That's a good idea. I just changed it. > > 6. 'sct' is a very short command name for /usr/bin ... have you >confirmed that it doesn't clash with any other packages in Debian? >You might have to set the priority to 'extra'. Is there any way to check if other packages have binaries called sct? A quick search of packages.d.o would seem to indicate there is not but is there a better way to check? > > 7. The language in d/copyright ("I doubt if it's copyrightable" etc.) >isn't appropriate. You need to determine whether or not it is >copyrightable and make a clear statement of that. That wording is not mine, but written by Ingo Thies, who would have copyright over the whitepoints data if it is copyrightable. Putting it there was the advice I got from debian-legal (see below). > > 8. This doesn't make sense (doesn't follow DEP-5 machine-readable >copyright file format) -- please check: > > Files: sct.c > Copyright: 2016 Ted Unangst >whitepoints data copyright 2013 Ingo Thies > > License: public-domain-sct and public-domain-colorramp This was based off of a discussion in debian-legal (starts here: https://lists.debian.org/debian-legal/2016/06/msg00018.html ). They did not explain how to properly format this except to say I should explicitly put the license grant from Ingo (the "I doubt if it's copyrightable" stuff) into d/copyright. I have no idea how to properly format this.Any ideas you have on how to write this up correctly are welcome. I've read DEP-5 but this is complicated because part of sct.c is copyright Ingo Thies but the rest is copyright Ted Unangst. > > 9. Please install the README into /usr/share/doc. Done. > 10. You're missing at least one build dependency. Please try building > in a clean sid chroot (see the pbuilder or sbuild tools). Yep. Was missing libx11-dev and libxrandr-dev (dpkg -S is the best :) ). I've fixed it now. Thanks for finding that. Thanks again for the review! I've now uploaded a new package to mentors. -- Jacob Adams GPG Key: AF6B 1C26 E2D0 A988 432B 94F4 24C0 2B85 B59F E5A9 signature.asc Description: OpenPGP digital signature
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
Hello Peter and Jacob, On Tue, Jul 05, 2016 at 11:19:55PM +0300, Peter Pentchev wrote: > Actually I don't see a problem with the machine-readable copyright > specification here; if you're referring to the fact that the contents > of the "Copyright" field is not in the usual "list of " > format, this is perfectly fine: the Copyright field is free-form text as > described in: > > > https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#copyright-field > > It is true that the examples in the copyright-format specification also > follow the "list of " format, but it is not codified in > the text. You're right -- thanks for your input. I was wrong to say that it doesn't satisfy DEP-5. However, I still think that the text could be more clearly laid out so the division of sct.c is obvious. You could write something like "remainder of C code copyright Ted Unangst". On Tue, Jul 05, 2016 at 04:51:51PM -0400, Jacob Adams wrote: > > 1. Could you explain why you are packaging your fork rather then the > >original? (This kind of thing should go in the ITP.) > > The original was released in a blog post as a sort of code dump > (http://www.tedunangst.com/flak/post/sct-set-color-temperature ) > I thought it would improve long term maintainability if there was a real > build system and active upstream. It's pretty hard to package software > without a tarball or VCS. If this is discouraged, I'd be happy to email > the real upstream about it, but I simply thought this would be easier > for everyone. > If I need to send a mail explaining this to the ITP I can do that. This is perfectly reasonable :) It would be good to append it to the ITP in case anyone else is wondering. > > > > 2. Could you put your Debian packaging in git, please? It makes > > reviewing easier. Perhaps as a 'debian' branch of your repo. > > Whoops. Actually I already do this in a different git repo and I just > filled out the relevant fields of d/control incorrectly. Thanks for > catching that! Great. Since you are the upstream maintainer and the (prospective) Debian package maintainer, is there any reason you're using two seprate git repos? You could just add a debian/ directory to your main repo. What you are doing is completely acceptable (and there are some Debian Developers who think that that is the way it should be done) but as someone who is the upstream maintainer and Debian maintainer (of git-remote-gcrypt), I find it a lot easier just to have one repo. Or you can have a debian branch containing the debian/ subdir and you can merge master into that branch periodically. > > > > 3. The formatting of the long description in debian/control is a bit > > strange. Please separate paragraphs using a line with the string " > > .". Probably best to wrap at 70 chars, too. > > I think I fixed this now. Sorry for the pedantry but you're missing a period at the end of line 19 :) > > 4. The wording of the long description could be improved. The first > > sentence isn't really a sentence -- it would be better to write "sct > > is a small C program to change the screen color temperature. It can > > be used to reduce or increase the amount of blue light produced by > > the screen." Please take another look at your wording :) > > Wow that description was a mess. I've fixed it up now and I think it's > much clearer. I agree. Good job. > > > > 6. 'sct' is a very short command name for /usr/bin ... have you > >confirmed that it doesn't clash with any other packages in Debian? > >You might have to set the priority to 'extra'. > > Is there any way to check if other packages have binaries called sct? A > quick search of packages.d.o would seem to indicate there is not but is > there a better way to check? There are several different ways, but the most convenient is probably the apt-file(1) tool. > > 7. The language in d/copyright ("I doubt if it's copyrightable" > > etc.) isn't appropriate. You need to determine whether or not it > > is copyrightable and make a clear statement of that. > > That wording is not mine, but written by Ingo Thies, who would have > copyright over the whitepoints data if it is copyrightable. Putting it > there was the advice I got from debian-legal (see below). Thanks for the link to the debian-legal thread. That was useful for me to read. I think you are good if you re-organise the Copyright: for sct.c as suggested above. > > 10. You're missing at least one build dependency. Please try building > > in a clean sid chroot (see the pbuilder or sbuild tools). > > Yep. Was missing libx11-dev and libxrandr-dev (dpkg -S is the best :) ). > I've fixed it now. Thanks for finding that. It builds now, and I also confirmed that it installs and works. -- Sean Whitton
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
On 07/05/2016 07:46 PM, Sean Whitton wrote: > Hello Peter and Jacob, > > On Tue, Jul 05, 2016 at 11:19:55PM +0300, Peter Pentchev wrote: >> Actually I don't see a problem with the machine-readable copyright >> specification here; if you're referring to the fact that the contents >> of the "Copyright" field is not in the usual "list of " >> format, this is perfectly fine: the Copyright field is free-form text as >> described in: >> >> >> https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#copyright-field >> >> It is true that the examples in the copyright-format specification also >> follow the "list of " format, but it is not codified in >> the text. > > You're right -- thanks for your input. I was wrong to say that it > doesn't satisfy DEP-5. However, I still think that the text could be > more clearly laid out so the division of sct.c is obvious. You could > write something like "remainder of C code copyright Ted Unangst". I've phrased it that way for now. Not sure I'm a huge fan of that wording, but I can't think of anything else that doesn't lose clarity. (I tried "everything else copyright..." but that seems too vague) > > On Tue, Jul 05, 2016 at 04:51:51PM -0400, Jacob Adams wrote: >>> 1. Could you explain why you are packaging your fork rather then the >>>original? (This kind of thing should go in the ITP.) >> >> The original was released in a blog post as a sort of code dump >> (http://www.tedunangst.com/flak/post/sct-set-color-temperature ) >> I thought it would improve long term maintainability if there was a real >> build system and active upstream. It's pretty hard to package software >> without a tarball or VCS. If this is discouraged, I'd be happy to email >> the real upstream about it, but I simply thought this would be easier >> for everyone. >> If I need to send a mail explaining this to the ITP I can do that. > > This is perfectly reasonable :) It would be good to append it to the ITP > in case anyone else is wondering. Done. > >>> >>> 2. Could you put your Debian packaging in git, please? It makes >>> reviewing easier. Perhaps as a 'debian' branch of your repo. >> >> Whoops. Actually I already do this in a different git repo and I just >> filled out the relevant fields of d/control incorrectly. Thanks for >> catching that! > > Great. Since you are the upstream maintainer and the (prospective) > Debian package maintainer, is there any reason you're using two seprate > git repos? You could just add a debian/ directory to your main repo. > > What you are doing is completely acceptable (and there are some Debian > Developers who think that that is the way it should be done) but as > someone who is the upstream maintainer and Debian maintainer (of > git-remote-gcrypt), I find it a lot easier just to have one repo. Or > you can have a debian branch containing the debian/ subdir and you can > merge master into that branch periodically. I separated the two because that's what the upstream guide in the debian wiki recommends. I can see the benefits of having it all in the same repo but for this package I think I will keep them separate for now. I might try the combined approach later if the separated one becomes too much of a burden. > >>> >>> 3. The formatting of the long description in debian/control is a bit >>> strange. Please separate paragraphs using a line with the string " >>> .". Probably best to wrap at 70 chars, too. >> >> I think I fixed this now. > > Sorry for the pedantry but you're missing a period at the end of line 19 :) Fixed. > >>> 4. The wording of the long description could be improved. The first >>> sentence isn't really a sentence -- it would be better to write "sct >>> is a small C program to change the screen color temperature. It can >>> be used to reduce or increase the amount of blue light produced by >>> the screen." Please take another look at your wording :) >> >> Wow that description was a mess. I've fixed it up now and I think it's >> much clearer. > > I agree. Good job. > >>> >>> 6. 'sct' is a very short command name for /usr/bin ... have you >>>confirmed that it doesn't clash with any other packages in Debian? >>>You might have to set the priority to 'extra'. >> >> Is there any way to check if other packages have binaries called sct? A >> quick search of packages.d.o would seem to indicate there is not but is >> there a better way to check? > > There are several different ways, but the most convenient is probably > the apt-file(1) tool. That's pretty useful thanks! According to apt-file there are no conflicts with /usr/bin/sct >>> 10. You're missing at least one build dependency. Please try building >>> in a clean sid chroot (see the pbuilder or sbuild tools). >> >> Yep. Was missing libx11-dev and libxrandr-dev (dpkg -S is the best :) ). >> I've fixed it now. Thanks for finding that. > > It builds now, and I also confirmed that it installs and works. > Thanks for all your help! -- Ja
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
On Tue, Jul 05, 2016 at 12:02:53PM +, Sean Whitton wrote: [snip] > 8. This doesn't make sense (doesn't follow DEP-5 machine-readable >copyright file format) -- please check: > > Files: sct.c > Copyright: 2016 Ted Unangst >whitepoints data copyright 2013 Ingo Thies > > License: public-domain-sct and public-domain-colorramp Actually I don't see a problem with the machine-readable copyright specification here; if you're referring to the fact that the contents of the "Copyright" field is not in the usual "list of " format, this is perfectly fine: the Copyright field is free-form text as described in: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#copyright-field It is true that the examples in the copyright-format specification also follow the "list of " format, but it is not codified in the text. I'm actually glad for that, and I'm actually glad that Dominique Dumont's libconfig-model-dpkg-perl package (usually used as "cme check dpkg") treats the Copyright field as such, because in one of the packages that I'm working on right now I have to include something like that: Files: lib/* Copyright: ... (c) 1998, 2001 The NetBSD Foundation, Inc. This code is derived from software contributed to The NetBSD Foundation by Charles M. Hannum. ... License: BSD-2-clause ...and there is no way to express that (and not lose information) without falling back on the free-form text idea. Of course, my interpretation might be wrong, in which case I would be glad for any advice on expressing the "derived from..." statement in a stricter format. G'luck, Peter -- Peter Pentchev r...@ringlet.net r...@freebsd.org p...@storpool.com PGP key:http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint 2EE7 A7A5 17FC 124C F115 C354 651E EFB0 2527 DF13 signature.asc Description: PGP signature
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
control: owner -1 ! control: tag -1 +moreinfo Dear Jacob, This looks like a nice alternative to redshift-gtk. Thanks for packaging it. I can't sponsor the upload, but I hope this review is useful to you. 1. Could you explain why you are packaging your fork rather then the original? (This kind of thing should go in the ITP.) 2. Could you put your Debian packaging in git, please? It makes reviewing easier. Perhaps as a 'debian' branch of your repo. 3. The formatting of the long description in debian/control is a bit strange. Please separate paragraphs using a line with the string " .". Probably best to wrap at 70 chars, too. 4. The wording of the long description could be improved. The first sentence isn't really a sentence -- it would be better to write "sct is a small C program to change the screen color temperature. It can be used to reduce or increase the amount of blue light produced by the screen." Please take another look at your wording :) 5. Have you considered calling the binary package 'sct'? That is what someone might guess when they want to install this with apt-get. 6. 'sct' is a very short command name for /usr/bin ... have you confirmed that it doesn't clash with any other packages in Debian? You might have to set the priority to 'extra'. 7. The language in d/copyright ("I doubt if it's copyrightable" etc.) isn't appropriate. You need to determine whether or not it is copyrightable and make a clear statement of that. 8. This doesn't make sense (doesn't follow DEP-5 machine-readable copyright file format) -- please check: Files: sct.c Copyright: 2016 Ted Unangst whitepoints data copyright 2013 Ingo Thies License: public-domain-sct and public-domain-colorramp 9. Please install the README into /usr/share/doc. 10. You're missing at least one build dependency. Please try building in a clean sid chroot (see the pbuilder or sbuild tools). -- Sean Whitton
Bug#829151: RFS: setcolortemperature/1.1-1 ITP
Package: sponsorship-requests Severity: wishlist Dear mentors, I am looking for a sponsor for my package "setcolortemperature" * Package name: setcolortemperature Version : 1.1-1 Upstream Author : Ted Unangst (I am upstream maintainer though) * URL : https://github.com/Tookmund/setcolortemperature * License : public domain Section : x11 It builds those binary packages: setcolortemperature - Set screen color temperature To access further information about this package, please visit the following URL: https://mentors.debian.net/package/setcolortemperature Alternatively, one can download the package with dget using this command: dget -x https://mentors.debian.net/debian/pool/main/s/setcolortemperature/setcolortemperature_1.1-1.dsc Changes since the last upload: setcolortemperature (1.1-1) unstable; urgency=medium * Initial release (Closes: #828028) -- Jacob Adams Sat, 04 Jun 2016 22:00:43 -0400 Regards, -- Jacob Adams GPG Key: AF6B 1C26 E2D0 A988 432B 94F4 24C0 2B85 B59F E5A9 signature.asc Description: OpenPGP digital signature