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

Re: ITR: scrotwm - dynamic tiling window manager



On 09/05/19 22:27 +0200, Andrea Bolognani said ...
> On Tue, 19 May 2009 19:46:46 +0530
> Y Giridhar Appaji Nag <appaji@debian.org> wrote:
> 
> > - It is not necessary that README.source be installed, README.source is for
> >   source packages only.
> 
> Fixed in collab-maint.
> 
> By the way, how am i supposed to upload a fixed version of the package to
> mentors.debian.net without adding a spurious changelog entry?

It is not necessary that you add a new dch entry.  Just upload a new version
of the package and mentors.d.n will replace the package it has with the new
version.

> > - Any reason why you call ./debian/rules unpatch and not just depend on the
> >   unpatch target?
> 
> So that calling `make clean' executes the clean target of the patched
> Makefile.
> 
> It isn't really needed at the moment, but were I to further patch the
> Makefile, I wouldn't need to modify the rules file. Moreover, since the
> build system is patched right before building, it seems correct to remove
> the patches right after clean.

That is right.  And many packages would want to do just this (use the patched
Makefile to clean) and they tend to do this with the clean target depending on
two targets, clean-patched and unpatch. 

I am certain that there are many situations where 'calling' debian/rules
recursively is not a good idea, but the scrotwm build system is rather simple
and not one of those.  Re-reading debian/rules isn't a lot of overhead either
so it is OK even if you leave this as is.

> > - Per policy, binary-arch and binary-indep are optional.  In your case there
> >   aren't complex arch/indep parts, so you can just have a binary: target.
> 
> I think you're mistaking binary-{arch,indep} for build-{arch,indep}. While
> the latter are optional, the former are required.

Ah, my mistake.  You are right.

> > - It might be a good idea to use a examples file rather than listing all the
> >   files installed as examples (you have more than one or two example files).
> 
> I tried creating an examples file, but neither debian/examples nor
> debian/scrotwm.examples did the trick. Can you confirm this?

A debian/examples file that has the following three lines works for me.

initscreen.sh
baraction.sh
screenshot.sh

In debian/rules, I changed ...

binary-arch: build
	[snip...]
	dh_installexamples initscreen.sh baraction.sh screenshot.sh

to

binary-arch: build
	[snip...]
	dh_installexamples

> > - apm executable in Debian is in /usr/bin and not in /usr/sbin.  Should you
> >   Patch baraction.sh appropriately?
> > 
> > - There are references to a "scrot" program in screenshot.sh, where does one
> >   find that program?

I later realised that there is a scrot package.

> I'm not sure it's worth it to patch the examples.

It usually is.  examples are 'more documentation' than documentation is.  When
I see an examples folder in /usr/share/doc/foo/examples, I tend to expect them
to work.

> I've patched the system-wide configuration file so that Scrotwm can be run
> without any kind of configuration, but the example files are provided just
> as guideline.

Thinking a bit more, In this particular case your examples have nothing to do
with scrotwm as such so I think it is OK to leave them unpatched.

> > - Since spawn_term uses x-terminal-emulator, you would want to Recommend
> >   x-terminal-emulator | xterm?
> 
> I Recommend dwm-tools for a similar reason, so it makes sense.
> Fixed in collab-maint.
> 
> > - Are you recommending xfonts-terminus package because terminus-medium is the
> >   default setting in scrotwm.conf?  Just wondering if you could downgrade that
> >   to a suggests.
> 
> Based on my tests, some fonts might not work. Moreover, as I explained above,
> I'd like the window manager to be usable right after installation without the
> need for further configuration.

OK.

That leaves us with just one TODO, the examples file.  Please fix that and
upload a new package to m.d.n and I'll sponsor an upload.

Cheers,

Giridhar

-- 
Y Giridhar Appaji Nag | http://appaji.net/

Attachment: signature.asc
Description: Digital signature


Reply to: