New PuTTY 0.54-0.3 rpms for review.

Dag Wieers dag at wieers.com
Sun Feb 22 01:23:27 UTC 2004


On Sun, 22 Feb 2004, Dag Wieers wrote:

> You've put my perl one-liner on one-line ;)) But you're doing 3 
> substitutions in one go and it becomes less obvious what you're doing if 
> you make it unreadable this way.
> 
> eg.
> 	%{__perl} -pi -e ' s,-O2,$RPM_OPT_FLAGS,g;s,/usr/local,%{_prefix},g;s,^(\t\$\(INSTALL_DATA\) -m 644 ps.+)$, ,' unix/Makefile.gtk
> 
> vs.
> 	%{__perl} -pi.orig -e '
> 	                s|-O2|%{optflags}|g;
> 	                s|/usr/local|%{_prefix}|g;
> 	                s|^(\t\$\(INSTALL_DATA\) -m 644 ps.+)$|#$1|;
> 	        ' unix/Makefile.gtk
> 
> Simplicity comes first, readability second. Putting a perl one-liner on a 
> single line you're not making this more simple, but sure less readable. 
> (Remember that when it gets fixed upstream you should verify that it can 
> be removed, if it takes you 1 minute to understand what it did...)

I forgot to add that you removed my comment:

	### FIXME: Disable missing pscp.1 and psftp.1. (Please fix upstream)

Of course you understand the 3rd substitution when you're reading it now, 
but there's no guarantee that the next time you're upgrading this package 
you'd still remember. Especially because it doesn't say that in fact 
you're diasbling 2 lines with the substitution.

Removing the comment will have a penalty the next time you want to upgrade 
it. The comments are there to help you (or anyone else) to remember things 
without having to go once again through the code to see what it is 
substituting.

(The FIXME comments are special in my case as they're indicated on the 
website so that other people now why they're missing and it may get fixed 
upstream too.)

--   dag wieers,  dag at wieers.com,  http://dag.wieers.com/   --
[Any errors in spelling, tact or fact are transmission errors]





More information about the fedora-devel-list mailing list