https://pantsbuild.org/ logo
Join Slack
Powered by
<@UB2J9BQA0>: so, the last change for <https://git...
# development
w
@hundreds-father-404: so, the last change for https://github.com/pantsbuild/pants/issues/10062 is going to remove the 
OptionsBootstrapper
param. there will be two commits: one that makes the OB unused, and then another that removes it everywhere.
thoughts 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
I think probably one. Two separate commits sounds helpful tho
w
sounds 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
Taking 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 
typing
. He isn’t back till Thursday, and hoping to land before then)
w
cool! will look.
h
Oh yes
w
had an edit in the #10062 thread, sorry
h
Oh, 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_value
, but better is to make that private, and have 
set_options(args=, env=)
w
yea.
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
Nope, the fixture creates a brand new instance every test
w
yea. ok. makes sense.
h
That’s why it’s crucial that you use a fixture, rather than, say, a module-level constant
w
alright… 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
seems close to graduating to an actual API.
💯 1
although no rush.
h
Agreed
w
ok, 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 QueryRules
tomorrow morning and landed it now?
h
Imo, the 
engine/internals
change is important too. It’s not a huge deal though to fix tomorrow, as there aren’t many imports of 
SessionValues
This 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
because unused Params in Queries are ignored
👍 1
h
Okay. I’m fine with landing now given that we want to do a beta release tonight
w
👍
h
For 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
yea, i think the original fix is fine for now
ftr: https://pantsbuild.slack.com/archives/C0D7TNJHL/p1600818099001900?thread_ts=1600714863.083400&amp;cid=C0D7TNJHL is effectively only because not all goal rules use all params that we provide in their query
👍 1
Open in Slack
PreviousNext
Powered by