Skip to content

Conversation

@joffrey-bion
Copy link
Contributor

This makes use of the MultiSourceReader provided by the graphQL library.
Using this reader allows to read a single document from multiple files,
while correctly associating line numbers and errors to their original
source file.

This doesn't bring any overhead because the method Parser.parseDocument(String) that was previously used already created a MultiSourceReader itself.
Note that the previous method didn't associate the source file name with the content used for parsing, while the new one does.

It allows to solve cross-reference problems with union types for instance.

Resolves:
#27

@kobylynskyi
Copy link
Owner

@joffrey-bion some tests are failing. Could you please take a look?

@joffrey-bion
Copy link
Contributor Author

joffrey-bion commented Mar 30, 2020

Yes, I'm fixing the tests.
It's because of the whitespace at the end of some test files.

I'll actually make a second PR for the removal of the trim() and the cleanup of the tests.

@codecov
Copy link

codecov bot commented Mar 30, 2020

Codecov Report

Merging #28 into master will increase coverage by 0.16%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #28      +/-   ##
============================================
+ Coverage     87.16%   87.33%   +0.16%     
- Complexity      217      220       +3     
============================================
  Files            29       29              
  Lines           522      529       +7     
  Branches         73       74       +1     
============================================
+ Hits            455      462       +7     
  Misses           44       44              
  Partials         23       23
Impacted Files Coverage Δ Complexity Δ
...om/kobylynskyi/graphql/codegen/GraphqlCodegen.java 86.58% <100%> (ø) 24 <0> (ø) ⬇️
...lynskyi/graphql/codegen/GraphqlDocumentParser.java 90.9% <100%> (+15.9%) 5 <4> (+3) ⬆️

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 9aeea21...01b1d7c. Read the comment docs.

@joffrey-bion
Copy link
Contributor Author

joffrey-bion commented Mar 30, 2020

I've made a separate PR, please take a look: #29

This one is based on the 29, so the diff will get cleaner when the other one is merged.

@kobylynskyi
Copy link
Owner

Awesome, thank you. #29 was merged.
Will merge this one after the following change will be done:
#28 (comment)

This makes use of the MultiSourceReader provided by the graphQL library.
Using this reader allows to read a single document from multiple files,
while correctly associating line numbers and errors to their original
source file.

This doesn't bring any overhead because the method Parser.parseDocument(String) that was previously used already created a MultiSourceReader itself.
Note that the previous method didn't associate the source file name with the content used for parsing, while the new one does.

It allows to solve cross-reference problems with union types for instance.

Resolves:
#27
@joffrey-bion
Copy link
Contributor Author

I added the requested change. Please don't hesitate to add any other comment.

@kobylynskyi
Copy link
Owner

@joffrey-bion thank you so much for the contribution.
Will release the change in 1.4.2 today

@kobylynskyi kobylynskyi reopened this Mar 30, 2020
@kobylynskyi kobylynskyi merged commit d31f5e1 into kobylynskyi:master Mar 30, 2020
@joffrey-bion
Copy link
Contributor Author

@kobylynskyi Well, thanks to you for being so quick to review, merge and release!

@joffrey-bion joffrey-bion deleted the multi-file branch March 30, 2020 14:18
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