Remove direct config references in thrift gen code.

Review Request #1839 — Created Feb. 26, 2015 and submitted

benjyw
pants
bcc7a40...
pants-reviews
dturner-tw, ity, jsirois, zundel
Remove direct config references in thrift gen code.

CI passes: https://travis-ci.org/pantsbuild/pants/builds/52212183

  • 0
  • 0
  • 1
  • 1
  • 2
Description From Last Updated
IT
  1. 
      
  2. this 'else' isnt aligned with 'if'

    1. I think that's intentional: https://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops

    2. Wow, that's new to me!

    3. Yep, for-else is very useful on occasion.

  3. s/write_generated_sources/iter_generated_sources

  4. 
      
DT
  1. 
      
  2. It's kind of a shame to remove this option -- what if someone just wants thrift gen to be verbose but not everything else?

    1. I think we discussed in a recent RB that it would be a good move to make -l (as in -ldebug) scoped so that we could do this for every task in a general way and not have to write code in every task to support it.

    2. All options are inherited by inner scopes, and can be overridden there. self.get_options().level is the log level in your scope, or the one inherited from the enclosing scope if not overriden.

      So, for example, ./pants -ldebug compile <tgt> sets debug logging globally, but ./pants compile -ldebug <tgt> sets it just in the compile scope, and ./pants compile.java -ldebug <tgt> sets it just in the compile.java scope.

      Not all tasks wire this up correctly yet, but scala_compile does, for example, if you want to see it in action.

      I think this is a lot better than having a bunch of haphazard, special-case verbosity flags scattered all over the codebase.

  3. 
      
ZU
  1. 
      
  2. Do we still need to store this in an attribute? I think we only have one reference left and it is in this method.

    1. There's another reference on line 208, but yes, we don't need it as we can access via self.context.config. Fixed.

  3. I think this can be PythonSetup(context.config) , also add a TODO to remove config reference from PythonSetup

  4. 
      
ZU
  1. Ship It!
  2. 
      
BE
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as 240b67e9809c48fd693e63e4e5122345faa67eec.

Loading...