On 07/30/12 15:31, Dan Price wrote:
On Fri 27 Jul 2012 at 04:33PM, Shawn Walker wrote:
On 07/27/12 15:57, Dan Price wrote:

I introduced a bug in our terminal handling when TERM isn't set or
is set to a bogus value.  Here's a fix:

https://cr.opensolaris.org/action/browse/pkg/dp/setupterm/

If possible I'd like to get this pushed today, so a speedy review
would be much appreciated...

src/modules/client/printengine.py:
   line 118: Downside:
     What is essentially an error is going to be sent to STDOUT
     instead of STDERR.

I'll just write it to stderr instead-- it seems more sane than
inventing yet another new exception type.  The other option would
be to print nothing, which would also be fine with me.

The silent approach may be best; it should be pretty obvious if the terminal type is screwed up since the progress tracking will be different. And if it isn't obvious, then the caller needs to check that.

src/po/POTFILES.in:
   Seems like we should be checking this during the build?
   (Separate bug obviously.)

I don't understand your comment, I'm sorry, I'm lacking context.
I did a build, it told me I had to add printengine to this file,
so I did...?

Oh. Sorry. I wasn't aware this was done at the prompting of one our build checks; for some reason I thought it was necessary before and you just discovered that. Nevermind.

Assuming we have a test somewhere that ensures the client fallsback
to the "basic" progress tracker if we can't use a fancier one, LGTM.

Yes, that happens every time you invoke pkg with a non-tty, and it's the
same exception class.  I have another fix coming (tomorrow, perhaps)
which will expand some of the edge-case testing of the printengine
and progress tracking.

Sounds fine to me,
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to