[pony-build] Context Failures

[C. Titus Brown]

On Thu, Apr 22, 2010 at 03:49:07PM -0400, Max Laite wrote:
> env
> 
> Tests:
> http://github.com/mlaite/pony-build/commit/3b3e53d15f0f8a83b927310507c8fb3095d813e9
> 
> pony_client:
> http://github.com/mlaite/pony-build/commit/150e3e1acab9cc78d13e85d181e3c705520b16a4

Hey Max,

a couple of comments :)

--

'init_status' is not very clear as a flag name goes.  Maybe 'initialized'?

--

How about creating a new context object of type 'Context' if context is None
in the 'do' function?  'Context' does a bunch of useful stuff, and it would
simplify all of your 'if' statements down to one 'if' at the beginning:

	if context is None:
		context = Context()

--

try/except with blanket 'except' and no reporting, e.g.

	try:
		# foo bar baz
	except:
		# cleanup with no reporting

is a Bad Idea.  I see a few problems with your code:

 - 'success' may still be set to True even if a c.run() call raises an
   exception.

 - there's no logging whatsoever; at the very least every 'except' branch
   should have some sort of log_error call.  Right now the excepts will
   catch everything, including genuine programming errors, silently!

-- 

Hmm, that's enough for now...

thanks,
--titus