refactor: migrate to use upstream GraphQL #57
Conversation
|
@namit-chandwani Amazing work so far. Also I see what you are fully aware of the challenges and have way to report the progress! Good work! |
@wtrocki Thank you! This means a lot coming from you. 😊 Also, can you please review the code changes whenever you're free? This will help me get an idea if I should proceed to work on the other imports with the same approach or not. |
|
Basically there is no good way to do it. If you start by imports then we can see if that works after all is finished. Alternative approaches would be to go to chririno fork and try to make it use latest graphql but not sure if that is faster |
Okay, got it.
Nice. I feel this would be a better way to do it Basically, my current approach involves comparing Mr. Chirino's fork with upstream for the imports that are being used in graphql-link and then modifying some code in the graphql-link repo in order to account for the changes made in his fork. This has worked well for me while replacing the But that's not the case for a few other imports. Like for example have a look at this commit on the graphql fork repo: Link to commit. So if I stick to my current approach then I'll have to copy all of these newly added methods from Mr. Chirino's fork into the relevant files of graphql-link repo (as these methods don't exist in the upstream graphql repo). And this is why I feel the alternative approach that you have mentioned is much better than mine. @wtrocki What's your take on this? Update 1: I have tried both approaches now and found the one which involves modifying Mr. Chirino's forked repo (instead of graphql-link repo) to be better and faster. Here's the PR for the same: (chirino/graphql#1). @wtrocki WDYT? |
|
@wtrocki Please check the comment above. Thanks! |
|
Modifications for fork are good. I will actually create this fork in aerogear so you can contribute against that? |
Yeah sure. |
An attempt to fix #41.
Checklist of Imports
Following is a list of imports that graphql-link uses from Mr. Chirino's fork of GraphQL.
I have planned to work on one import at a time and update the checklist as soon as that import is sucessfully converted to an import from the upstream GraphQL repo.
Further comments
The commit currently attached to this PR is an attempt to replace the import:
"github.com/chirino/graphql/qerrors"with the import:"github.com/graph-gophers/graphql-go/errors"by making some changes in the relevant files.Once this commit change gets approved, I'll move on to work on any of the remaining imports from the checklist above, by adding further commits to this PR itself.
I wasn't sure of the exact approach that was to be followed to perform the migration, so I tried this.
If there is any fault in my approach then please let me know, I would be happy to make the necessary changes according to the requirements.