Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fuse hydrated and unhydrated Struct parsing #7523

Merged

Conversation

Projects
None yet
3 participants
@stuhood
Copy link
Member

commented Apr 10, 2019

Problem

As described in #4535, there are a bunch of different layers to our BUILD file parsing currently, mostly because we were working up from a set of powerful experiments that @jsirois started, and down from the existing (legacy) BuildGraph model.

Now that @union and union_rule are in place, it's almost time to take a holistic look at what our target API should look like. Before doing that, we can remove at least one of the parsing layers to simplify things a bit.

Solution

Merge the first two layers described in #4535 (UnhydratedStruct and HydratedStruct (nee TargetAdaptorContainer)) into HydratedStruct. This removes one datatype and one rule, and prunes some implementation-specific test code.

Result

Fewer rules/nodes, one less concept, and slightly simpler code.

@stuhood

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

The two commits are useful to review independently.

@stuhood stuhood requested a review from jsirois Apr 10, 2019

@illicitonion
Copy link
Contributor

left a comment

Nice simplification :)

@benjyw

benjyw approved these changes Apr 10, 2019

Copy link
Contributor

left a comment

Neat! Just a couple of requests for clarification in the code.

Show resolved Hide resolved src/python/pants/engine/build_files.py
Show resolved Hide resolved src/python/pants/engine/build_files.py Outdated
@stuhood

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

.. a lot of unexpected timeouts in this run. Hoping that that was infrastructure issues.

@stuhood stuhood force-pushed the twitter:stuhood/fuse-hydrated-and-unhydrated branch from 5fc7778 to 741be7c Apr 10, 2019

@stuhood stuhood merged commit fec176e into pantsbuild:master Apr 11, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stuhood stuhood deleted the twitter:stuhood/fuse-hydrated-and-unhydrated branch Apr 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.