Re: RFS: faxfrontend
On Oct 21, 2010, at 9:44 PM, René Mayorga <rmayorga@debian.org> wrote:
> On Thu, Oct 21, 2010 at 08:37:12PM -0400, Tyler Gates wrote:
>> Dear mentors,
>
> Hi Tayler!,
>
Hi Rene. Let me first start out by saying this and qfaxreader are my first attempts at making a debian package. In fact I'm using Ubuntu but understand this is were to start to get your package included in their repositories :)
>>
>> * Package name : faxfrontend
>
> Some comments about your package:
> + You need to close an ITPi[1], in this case there is one old ITP, you should
> re-open it and close it on you d/changelog (#383195).
Sorry, you've lost me here..
>
> + Use DH_VERBOSE on d/rules is usefully only for debug options, there is a
> reason why to include it on you package?.
I was using it for debugging and figured it wouldn't hurt to just leave it.
>
> having debian/$(NAME)/etc/sudoers.d/faxfrontend IMHO looks a bit creepy, I do
> not want to have a package on my system that requires a user with a paswordless
> sudoers entry added by default, so far the package include this file commented.
I know I hate it and I looked for ways to avoid it but there's really no other way.
>
> Also I see that there is a warning for this on postinst configure, it would not
> be better to use debconf here?
Not entirely familiar with debconf or what exactly you had in mind but if you are talking about user interaction to enable or disable the sudo, I personally prefer to stay away from those methods.
>
> + d/control Standards-Version: 3.8.3 can be updated.
All right. I wasn't sure if that would be ok: I had generated using dh_make and that is what it spit out as.
>
> + debian/patches/ might need some comment regarding what the patch do.
The patches themselves? Sounds good, I hadn't thought about it.
>
> + there are some more copyright mentioned on the code that the one listed on
> d/copyright: fax4cups/hylafax.in:# Copyright (C) 2001-2002 Sebastiano Vigna
> <vigna@acm.org>
Yeah it's kind of scattered all over the place. I may have gotten that one from his website...
>
> + NEWS and TODO files are empty, why they are included on d/docs?
Hadn't checked them, thanks for pointing it out.
> + You are including also readme.pics/*, but the only file linking those files
> is A-README.html which is not included on your package.
Ha! Good eye! As you can see I wasn't to concerned about checking the miscellaneous stuff ;)
>
>
> I was also wondering why you are using Architecture: i386, I don't have another
> architecture available right now to tests this.
I guess coming from gentoo I had the logic of testing it working on 386 gets the 386 stamp of approval but now I know those keywords are different....
>
> Those are my comments so far, I did not dig more on the package.
Don I need to make these changes and resubmit the package?
Thanks for all your help!
>
> [1] http://www.debian.org/devel/wnpp/
>
>
> Cheers
> --
> René
Reply to: