On 04/13/10 11:16 AM, Tim Foster wrote:
Hi all,

As promised, I'm resubmitting this change for code review now that the
gate has branched.  I've addressed the comments Danek&  Shawn had made
below (thanks again!), have retested and have the updated webrev at:

  http://cr.opensolaris.org/~timf/1797-contents


This LGTM. Only tiny tiny nit I have is that the variable named "printed_headers" seems misleading since it can be true even when the headers haven't been printed. Maybe 'printed_output'?
Anyway, not a big deal either way.

Brock

My last mail concerning this change was at:
http://mail.opensolaris.org/pipermail/pkg-discuss/2010-March/021470.html

which I've reproduced below for convenience.

        cheers,
                        tim


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.

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

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

Reply via email to