[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: lockrun



On Thu, May 29, 2008 at 07:04:30PM -0500, Raphael Geissert wrote:
> debian/copyright:
> > Debianized-By: Noah Slater <nslater@bytesexual.org>
> > Debianized-Date: Sat, 08 Mar 2008 22:58:05 +0000
>
> You should read the wiki page again now, those have a different name now.

Heh, I was the one who made that change to the proposal so I should have picked
up on this sooner. Thanks, fixed now.

> > License: PD
[...]
> > License: GAP
[...]
>
> I am not an expert in this, but AFAIK those shouldn't be in separate lines.

No, blank lines are allowed in the debian/copyright file.

> debian/patches/command-option.patch:
> > -               else if ( STRMATCH(arg, "-T") ||
> STRMATCH(arg, "--maxtime"))
> > +               else if ( STRMATCH(arg, "-t") ||
> STRMATCH(arg, "--maxtime"))
>
> Is this really needed? I don't think you should be so intrusive. Unless, of
> course, upstream agrees to make that change and does it at upstream too.

Okay, good point. I have contacted the upstream, will wait his response on this.

If he rejects my patch, or that part of it, I will remove naturally.

> debian/rules:
> > include /usr/share/cdbs/1/rules/buildcore.mk
> That line is useless, as including debhelper.mk already takes care of that.
> If you include that file before debhelper.mk, not all parts of debhelper are
> used, as cdbs doesn't know that you are later going to load debhelper.mk

Good point, thanks. Fixed.

> > PACKAGE_NAME = lockrun
> > ...
> Isn't the cdbs-provided var more than enough? or is it bogus?

Wow, I had not seen those before, thanks. Fixed.

> > SOURCE_URI=http://www.unixwiz.net/tools/lockrun.c
> Have you considered using debian/copyright's Original-Source-Location:
> instead of hard-coding the uri twice? ;-)

Yes, I considered this but these two things serve separate purposes.

The Original-Source-Location is typically used for the homepage of upstream
software whereas the SOURCE_URI in this case is the actual source file.

> > else
> > CC=cc
> > endif
>
> Didn't you say
> > make already defines one by default:
> ?
> Just strip the 'else' and 'CC=cc' lines.

Sorry, I don't understand this bit.

My debian/rules doesn't have these lines.

> >         rm -f lockrun lockrun.1
> better written as $(RM) lockrun lockrun.1
> make's default $(RM) already sets -f.
>
> >         rm -fr $(PACKAGE_DIRECTORY)
> Like above, but also set -r, e.g. $(RM) -r ...

What is the value in doing this? As I understand it, this is only used by
implicit rules in make and so isn't really useful for this scenario as it's not
going to be overridden by anything.

> Your package still FTBFS twice in a row because of:
> > sed -i "s/@version@/$(PACKAGE_VERSION)/" lockrun.c
> not being reverted.

I have fixed this now.

The package has been re-uploaded to mentors.debian.net.

Thanks,

-- 
Noah Slater - Bytesexual <http://bytesexual.org/>


Reply to: