Hi all,

Just block-replying to the comments that arrived in when I was on
vacation.  Thanks for all of these, they're much appreciated.

On Tue, 2010-02-23 at 12:22 +0000, Tim Foster wrote:
> http://cr.opensolaris.org/~timf/1797-contents

[ snip my talking about performance of sorted raw contents, vs. not
 (and completely missing the point). ] 

On Wed, 2010-02-24 at 10:08 -0800, Danek Duvall wrote: 
> See bug 4315: contents -m shouldn't sort.

Aha.  I understand - thanks.

On Tue, 2010-02-23 at 14:57 -0600, Shawn Walker wrote:
> lines 2296-2314: You may want to ask BJ about the string addition; 
> we're pretty late in the release cycle.

BJ came back with an opinion: he reckons we're too late to make this
change given that it's an old bug (and not a good a good enough case to
warrant bothering the translators)   With that in mind, I've removed
1797 from this wad.

On Wed, 2010-02-24 at 10:41 -0800, Danek Duvall wrote: 
> Rather than buffering up all the contents lines to find whether there were
> any, and then emitting them, you should probably just print the header in
> the loop, and only once

That's a lot nicer - thanks.  I'll make that change in my local
workspace for 1797 and submit them for a quick re-review once we're past
this release.

On Wed, 2010-02-24 at 14:58 -0600, Shawn Walker wrote: 
> >>      lines 2310-2311:  The formatting seems odd here.
> >
> > That was intentional - I was trying to highlight the differences between
> > the plural and singular forms, thinking it was easier to read this way -
> > I can wrap all lines to 90 chars if you'd prefer?
> 
> Wrapping at 80 chars is nice

I had thought you were talking about the formatting of the code, rather
than its output.  Wrapping the text as displayed would be a worthwhile
change alright.

> , but my one thought was about adding some 
> line-wrapping to the message manually (hard line-breaks).  If you did 
> that, then I'd just do something like usage() does in client.py.

Yep, I'll make that change too for the 1797 re-review.  For now, 
I've removed that bug from the wad of changes, and have the new webrev
at:

http://cr.opensolaris.org/~timf/14625,13813,13814-aesthetics

        cheers,
                        tim

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to