Skip to content

Conversation

@joffrey-bion
Copy link
Contributor

@joffrey-bion joffrey-bion commented Mar 31, 2020

This didn't require any change in the current GraphqlCodegen implementation after all.
We can actually just use the SchemaFinder from the caller code (plugin) to get a list of schemas, and then call the generator with that list.

Resolves:
#30

@codecov
Copy link

codecov bot commented Apr 1, 2020

Codecov Report

Merging #32 into master will increase coverage by 0.52%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #32      +/-   ##
============================================
+ Coverage     87.33%   87.86%   +0.52%     
- Complexity      220      231      +11     
============================================
  Files            29       30       +1     
  Lines           529      552      +23     
  Branches         74       76       +2     
============================================
+ Hits            462      485      +23     
  Misses           44       44              
  Partials         23       23              
Impacted Files Coverage Δ Complexity Δ
...lynskyi/graphql/codegen/supplier/SchemaFinder.java 100.00% <100.00%> (ø) 11.00 <11.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 510a61c...1658d54. Read the comment docs.

@kobylynskyi kobylynskyi self-assigned this Apr 1, 2020
@kobylynskyi kobylynskyi self-requested a review April 1, 2020 12:38
Copy link
Owner

@kobylynskyi kobylynskyi left a comment

Choose a reason for hiding this comment

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

@joffrey-bion this looks great! 🏆
BTW, I see you are still making some changes to the tests, so please let me know when I can merge this.

@joffrey-bion
Copy link
Contributor Author

I'm done with the tests, there was an issue about file traversal order. On the CI the order was not the same.
I realized that the depth-first order of the spec didn't guarantee anything about same-level order.

I fixed the tests by sorting the returned schemas alphabetically

@kobylynskyi kobylynskyi merged commit 9af4149 into kobylynskyi:master Apr 1, 2020
@joffrey-bion
Copy link
Contributor Author

Thanks a lot for merging! I'll make changes to the plugins once a new version of the core lib is released

@kobylynskyi
Copy link
Owner

I will try to work on #33 first if you don't mind. So that you can make changes in the same repo.

@joffrey-bion
Copy link
Contributor Author

@kobylynskyi sure that's totally fine. It's really nice that you're doing this. Thanks a lot for your reactivity and availability.

@kobylynskyi kobylynskyi added this to the 1.5.0 milestone Apr 3, 2020
@joffrey-bion joffrey-bion deleted the 30-directory-support branch April 3, 2020 22:54
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.

2 participants