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.

Sign up for GitHub

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

Jump to bottom

Add fragment builder #32

Merged
merged 30 commits into from
Feb 8, 2020
Merged

Add fragment builder #32

merged 30 commits into from
Feb 8, 2020

Conversation

smkhalsa
Copy link
Member

No description provided.

@klavs
Copy link
Contributor

klavs commented Jan 18, 2020

@smkhalsa would you be able to make a separate PR containing the flutter example?

My intention is to limit the discussion on this PR to the fragment builder, and discuss flutter example on the other PR. If you could, it would be nice if that PR did the same without using fragments. That would nicely show the improvements made by having this fragment builder.

klavs
klavs reviewed Jan 18, 2020
View reviewed changes
gql_build/example/README.md Outdated Show resolved Hide resolved
@smkhalsa smkhalsa mentioned this pull request Jan 18, 2020
Merged
@smkhalsa
Copy link
Member Author

@smkhalsa would you be able to make a separate PR containing the flutter example?

See #33

@smkhalsa smkhalsa mentioned this pull request Jan 18, 2020
Merged
@klavs
Copy link
Contributor

klavs commented Jan 19, 2020

Please rebase

@smkhalsa
Copy link
Member Author

Please rebase

@klavs done

klavs
klavs suggested changes Jan 26, 2020
View reviewed changes
Copy link
Contributor

@klavs klavs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adjust code formatting issues.

gql_example_flutter/test/widget_test.dart Outdated Show resolved Hide resolved
gql_example_flutter/pubspec.yaml Outdated Show resolved Hide resolved
gql_example_flutter/lib/src/pokemon_detail/pokemon_detail.dart Outdated Show resolved Hide resolved
gql_code_builder/lib/src/operation/data.dart Outdated Show resolved Hide resolved
gql_code_builder/lib/src/operation/data.dart Outdated Show resolved Hide resolved
gql_code_builder/lib/src/operation/data.dart Outdated Show resolved Hide resolved
@smkhalsa smkhalsa requested a review from klavs January 28, 2020 21:10
@smkhalsa
Copy link
Member Author

@klavs I've integrated this into my moderately sized project, and it appears to be working well, including deeply nested fragments, inline fragments, and fragment spreads.

@smkhalsa smkhalsa mentioned this pull request Jan 30, 2020
Closed
@smkhalsa
Copy link
Member Author

smkhalsa commented Feb 5, 2020

@klavs I just rebased.

Anything else that needs to be done before this can be merged?

@klavs
Copy link
Contributor

klavs commented Feb 5, 2020

I'll try to do a review by the end of the week.

Some functions have quite many args, could you refactor to named/optional args?

@smkhalsa
Copy link
Member Author

smkhalsa commented Feb 5, 2020

Some functions have quite many args, could you refactor to named/optional args?

Ok. I converted the two functions that have more args to optional named.

@smkhalsa smkhalsa mentioned this pull request Feb 5, 2020
Closed
@klavs
Copy link
Contributor

klavs commented Feb 8, 2020

Thank you for your contribution and the almost one month long commitment to get this done.

@klavs klavs merged commit f7ddac4 into gql-dart:master Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@klavs klavs klavs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@smkhalsa @klavs