Skip to content

Refactor schema representation and code generation #308

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

Merged
merged 93 commits into from
May 15, 2020
Merged

Conversation

tomhoule
Copy link
Member

@tomhoule tomhoule commented Feb 5, 2020

This is a huge refactoring based on an idea to model the schema with a proper normalized representation. The results so far are promising in terms of code clarity and maintainability. At the moment, this is far from complete.

Previously, name resolution, query validation and code generation all happened in one pass. In this PR, these three are cleanly separated, leading to much more understandable code in my opinion.

@h-michael

This comment has been minimized.

@tomhoule

This comment has been minimized.

@tomhoule tomhoule force-pushed the schema-graph branch 5 times, most recently from 10236d8 to f2b3eee Compare February 14, 2020 03:05
@pickfire

This comment has been minimized.

@tomhoule

This comment has been minimized.

@yuyoyuppe

This comment has been minimized.

@pickfire

This comment has been minimized.

@yuyoyuppe

This comment has been minimized.

@tomhoule

This comment has been minimized.

@yuyoyuppe

This comment has been minimized.

@pickfire

This comment has been minimized.

@tomhoule

This comment has been minimized.

@tomhoule

This comment has been minimized.

@yuyoyuppe

This comment has been minimized.

@tomhoule

This comment has been minimized.

@yuyoyuppe

This comment has been minimized.

@pickfire

This comment has been minimized.

@tomhoule tomhoule force-pushed the schema-graph branch 7 times, most recently from 618f2ce to ecd34f2 Compare March 20, 2020 21:28
@tomhoule
Copy link
Member Author

I'm going to merge this soon, since it my view it's a clear improvement, and it's blocking me from working on other issues.

@mathstuf
Copy link
Contributor

Sounds good to me; I won't block it, but I also haven't gone through with a fine-tooth comb either. The broad strokes I see when skimming look good though.

@tomhoule
Copy link
Member Author

Yes it's way too big to expect a thorough review - I wish I'd managed to break it into reviewable parts, but it started as an experiment and was that for a long time, and by the time I found a structure that worked very nicely there were a bunch of improvements and refactorings mixed together. And I don't have the spare time to make it much tidier unfortunately. Thanks for taking a look though :)

@tomhoule tomhoule merged commit db63a99 into master May 15, 2020
@mathstuf mathstuf deleted the schema-graph branch May 15, 2020 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants