[25992] in Source-Commits
Re: /svn/athena r25317 - trunk/debathena/config/cupsys-config/debian
daemon@ATHENA.MIT.EDU (Jonathan Reed)
Sun Jul 31 07:58:33 2011
Mime-Version: 1.0 (Apple Message framework v1084)
Content-Type: text/plain; charset=us-ascii
From: Jonathan Reed <jdreed@MIT.EDU>
In-Reply-To: <alpine.GSO.1.10.1107302149120.7526@multics.mit.edu>
Date: Sun, 31 Jul 2011 07:58:25 -0400
Cc: source-commits@mit.edu
Message-Id: <3893D785-E220-466C-BA84-B6D9E651074C@mit.edu>
To: Benjamin Kaduk <kaduk@mit.edu>
Content-Transfer-Encoding: 8bit
>> + [ Jonathan Reed ]
>> + * Stop BrowsePolling and just add the mitprint printer. Remove any
>> + printers added by cluster-cups-config at upgrade time
>> + * Replace/Conflict/Provide debathena-cluster-cups-config
>> + * Bump standards-version
>
> Shouldn't there be a trac closer here -- this is #842, is it not?
There should. I will add one.
>> +add_printers() {
>> + rm -f $PRLIST
>> + for p in $PHAROS_PRINTERS; do
>> + lpadmin -p $p -E -v lpd://mitprint.mit.edu/bw \
>
> I'm not really convinced that the PHAROS_PRINTERS abstraction meshes well with this loop. Are we ever going to want to have two names for the bw queue? with entirely the same options, now that we've moved away from named queues for each physical printer hardware?
> Certainly this will not work as-is when we get color capability.
You are correct, we will have to rewrite configure-athena-printers for color.
>
>> + -D "Pharos (Monochrome)" \
>> + -L "Release jobs from any Pharos printer" \
>> + -o printer-is-share=false \
>> + -m drv:///hpijs.drv/hp-laserjet_9050-hpijs-pcl3.ppd
>> + if [ $? != 0 ]; then
>> + echo "FAILED to add Pharos printer $p"
>> + else
>> + echo "Added Pharos printer $p"
>> + echo "$p" >> $PRLIST
>
> The pedant in me wants the past-tense notice to go after the actual addition (but I wouldn't have said anything if I wasn't writing mail anyway).
It is correct, after that point it has been added, but has not yet added to the list of printers to remove in the future... So the tense is correct, but I will address this when I rewrite configure-athena-printers.
>> + fi
>> + done
>> + for a in $ATHENA_PRINTERS; do
>> + # Don't clobber queue, this is -cluster
>> + if add-athena-printer $a; then
>
> You're mixing tabs and spaces, here.
I'll fix that.
> I'm also a little confused -- is the sense of the comment reversed? It looks like we *are* clobbering any existing queues of the same name ...
No, we're not, because we're not passing -f. add-athena-printer will fail if there's a local queue of the same name.
>>
>> + if ! /etc/init.d/$rcname status; then
>
> I seem to recall that we just fixed code that presumed cups status ever returned nonzero elsewhere ... we should perhaps have some common library routines for these things, per #985.
Good catch, I'll fix that. We do have some cleanup to do (for example, restart_cups in cupsys-config no longer needs to do its sketchy browsepolling stuff) and can then use a common routine for this (though obviously not necessarily in the maintainer scripts of the package that provides said routine.
>
> Mixing tabs and spaces here, too.
Noted.
>> +# dh_installdeb will replace this with shell code automatically
>> +# generated by other debhelper scripts.
>> +
>> +#DEBHELPER#
>
> Does it matter whether the #DEBHELPER# goes at the start or the end?
I don't think so?
