https://pantsbuild.org/ logo
Join Slack
Powered by
Bikeshed: We have the file `pants/python/binaries....
# development
h
Bikeshed: We have the file 
pants/python/binaries.py
, which defines 
PythonBinary
+ 
PythonBootstrapSubsystem
.
I'm moving 
PythonBinary
to 
core/util_rules/system_binaries
, so now we're left with a dir w/ just that file and one class definition + accompanying logic. Feels wrong. Two thoughts on where to move this: 1. 
core/subsystems/python_bootstrap.py
. That would be a new folder. We could put things like anyonymous telemetry there? which imo doesn't really belong in 
pants/goal
2. 
backend/python/util_rules/python_boostrap.py
. That's a lie, it's used by every backend
I'm pretty sure 
core/subsystems
would be the right place, but the thing holding me back is it's unlikely plugin authors would need to import 
PythonBootstrapSubsystem
or 
AnynomyousTelemetry
etc. At least for 
util_rules/
, it's meant to be useful things for plugin authors
cc @ancient-vegetable-10556
f
core/util_rules/python_binaries.py
?
(or singular)
h
That feels even worse because this isn't really a util for plugin authors. It truly is a subsystem that is needed for Pants's core
a
Do we have a namespace for actual internals yet? 
internals/subsystems/python_bootstrap.py
would make sense to me
and then we define 
internals
as a package that is here be dragons for plugin authors
h
We don't. Generally we've been treating 
core/
and 
engine/
that way, including 
engine/internals
This definitely doesn't belong in `engine/internals`: it's not foundational to Pants
a
Right, I guess I’m wondering what expectations we’ve been mis-setting with 
core
that is giving you cause for concern?
h
that is giving you cause for concern?
I'm pretty sure it's okay to put in 
core/subsystems
. It's not wrong to import the type 
PythonBootstrapSusbsystem
, and in fact, the Python backend needs to. I do think 
core/util_rules
is wrong tho, as it's not really a util like 
archive.py
or 
external_tool.py
is. So, my only hesitation is "are we okay with adding a new folder to 
core/
?"
I'll make this new file name be a dedicated change so it can be properly bikeshedded / documented in a PR
a
Yes, definitely
🙌 1
having a 
/subsystems
subpackage is an established pattern
👍 1
no reason not to do it
h
Open in Slack
PreviousNext
Powered by