#development
Title
w
witty-crayon-22786
09/21/2020, 7:01 PM@hundreds-father-404: so, the last change for https://github.com/pantsbuild/pants/issues/10062 is going to remove the
OptionsBootstrapperthoughts on whether they should be in one PR or two PRs?
i’m sortof thinking one PR, to make it easier to revert, but am going to do a CI run with only the first commit to check that that is fine first.
h
hundreds-father-404
09/21/2020, 7:02 PMI think probably one. Two separate commits sounds helpful tho
w
witty-crayon-22786
09/21/2020, 7:08 PMsounds good, thanks
hm. this is more complicated than i realized. will do a bit of design on https://github.com/pantsbuild/pants/issues/10062
👍 1
@hundreds-father-404: https://github.com/pantsbuild/pants/issues/10062#issuecomment-696389789. @average-vr-56795 maybe you have thoughts too?
h
hundreds-father-404
09/21/2020, 9:36 PMTaking a look. Meanwhile, if you want a quick break, Danny and I thrilled that we got MyPy working with Pex! https://github.com/pantsbuild/pants/issues/10819
(John acked the whole project; we met his main requirement to not vendor
typingw
witty-crayon-22786
09/21/2020, 9:36 PMcool! will look.
you mean https://github.com/pantsbuild/pex/pull/1032 i’m guessing?
h
hundreds-father-404
09/21/2020, 9:40 PMOh yes
w
witty-crayon-22786
09/21/2020, 9:56 PMhad an edit in the #10062 thread, sorry
h
hundreds-father-404
09/21/2020, 9:59 PMOh, even better with the mutability (which is funny for me to say - I hate mutability haha)
It’s more consistent with everything else.
I think it’s fine to have
set_session_valueset_options(args=, env=)w
witty-crayon-22786
09/21/2020, 10:00 PMyea.
i don’t know how pytest fixtures work though. is that a landmine…?
ie, would you end up mutating the fixture?
… i guess since it already has a build root associated, that’s already a potential issue
h
hundreds-father-404
09/21/2020, 10:01 PMNope, the fixture creates a brand new instance every test
w
witty-crayon-22786
09/21/2020, 10:02 PMyea. ok. makes sense.
h
hundreds-father-404
09/21/2020, 10:02 PMThat’s why it’s crucial that you use a fixture, rather than, say, a module-level constant
w
witty-crayon-22786
09/21/2020, 10:04 PMalright… seems pretty clear cut then. i’ll look into it. i’m probably going to make it private/ugly.
❤️ 1
independently but relatedly,
@_uncacheable_rule💯 1
although no rush.
h
hundreds-father-404
09/21/2020, 10:05 PMAgreed
w
witty-crayon-22786
09/22/2020, 11:25 PMok, posted for review… despite what i said above, i ended up applying the minimum possible change to TestBase and RuleRunner: https://github.com/pantsbuild/pants/pull/10827
❤️ 1
@hundreds-father-404: hmmm, so it went green. would you mind if i followed-up to fix the
TODO: Once the OptionsBootstrapper has been removed from all relevant QueryRulesh
hundreds-father-404
09/22/2020, 11:40 PMImo, the
engine/internalsSessionValuesThis isn’t a bug? It looks like things still work? I’m not sure why
should you then pop this value from the inputs?
w
witty-crayon-22786
09/22/2020, 11:41 PMbecause unused Params in Queries are ignored
👍 1
h
hundreds-father-404
09/22/2020, 11:42 PMOkay. I’m fine with landing now given that we want to do a beta release tonight
w
witty-crayon-22786
09/22/2020, 11:42 PM👍
h
hundreds-father-404
09/22/2020, 11:43 PMFor MyPy, I don’t think I’m gonna have the Py2 fix in time for the release tonight. Should I land the other inferior fix for it? And do the improved fix after?
Right now, if you have any Py2 code, you can’t even run MyPy. The original fix at least avoids things erroring
But will be noisy diff
w
witty-crayon-22786
09/22/2020, 11:45 PMyea, i think the original fix is fine for now
ftr: https://pantsbuild.slack.com/archives/C0D7TNJHL/p1600818099001900?thread_ts=1600714863.083400&cid=C0D7TNJHL is effectively only because not all goal rules use all params that we provide in their query
👍 1