witty-crayon-22786
07/31/2020, 6:44 PMhappy-kitchen-89482
07/31/2020, 7:05 PMwitty-crayon-22786
07/31/2020, 7:17 PMhundreds-father-404
07/31/2020, 7:19 PMField
, or a special cased property? A tricky thing with Field
is that the user must include it in the target’s definition, i.e. the class property core_fields = ()
.Target
sounds like a nice change, though. Notably, it would allow us to consistently have better error messages by pointing to the actual BUILD file. In the future, we could even include the line number of the BUILD filewitty-crayon-22786
07/31/2020, 7:22 PMField
. the implication is that a Target
always “has” BuildFileSources
, which aligns with the Address
changehundreds-father-404
07/31/2020, 7:28 PMField
, you wouldn’t be able to confirm it always has BuildFileSources
.
It was intentional that a target can have zero fields registered on it - we don’t want to force you to use any specific field. You’re encouraged, but not required, to use COMMON_TARGET_FIELDS
.
If you don’t want to special case that the field is always no matter what registered, you’d need to add an assert
to the constructor to validate that BuildFileSources
is included in core_fields
. At that point, I’d recommend calling out the special casing by instead having BuildFileSources
be an actual distinct property that you access via tgt.build_file_sources
, rather than tgt[BuildFileSources]
.
--
The other tricky thing is that you need a consistent way to calculate that BuildFileSources
. One place that complicates things is testing, as right now you can create a Target
like a normal Python object without any BUILD file actually existing on disk.
This problem is what’s stopped me in the past with a side project to merge Address
with BFA
. I think 1.5 months ago, I realized how much nicer it would be if we always had the information stored in BFA
, just without the clunkiness of that actual type. I found it harder to do than I expected.witty-crayon-22786
07/31/2020, 7:31 PMThe other tricky thing is that you need a consistent way to calculate thatyou can do this with any Sources field without the files existing on disk, as long as you don’t try to hydrate them. One place that complicates things is testing, as right now you can create aBuildFileSources
like a normal Python object without any BUILD file actually existing on disk.Target
Sources
.hundreds-father-404
07/31/2020, 7:33 PMbut… it feels like making it a field is less of a special case.The difference is that it sounds like you expect this field to always exist, no matter what. That is in itself a special case. Even with
Sources
, that is not always used. For example, python_requirement_library
has no Sources
field.witty-crayon-22786
07/31/2020, 7:33 PMname
is a “required” field (that “turns into” your `Address`…? very related, i think…)hundreds-father-404
07/31/2020, 7:34 PMname
is not a Field
. We actually never expose that via the Target API. The rule to go from TargetAdaptor -> Target
will pop the name
field from the kwargs
and convert it into an Address
, which then gets passed to the constructor of Target
as a special valuewitty-crayon-22786
07/31/2020, 7:35 PMTarget
feels integrally related to which BUILD file it comes from. i expect it’s possible, but whether it’s an extension of the special case or not is an open question