Skip to content
This repository has been archived by the owner on Nov 20, 2020. It is now read-only.

Subscribe to more #91

Merged
merged 7 commits into from
Jan 8, 2020

Conversation

Yakimych
Copy link
Contributor

This PR is a continuation of the work done by @MargaretKrutikova on subscribeToMore. I've added examples to demonstrate the functionality and rearranged the code a bit. I am also planning to add more examples after migrating from Graphcool to Hasura: #90

There are a few questions I am not sure about:

  • is it ok to update the devDependency to bs-platform v7 in the library itself (e260c52#diff-b9cfc7f2cdf78a7f4b91a753d10865a2)?
  • is it ok to use BuckleScript 7 for the examples? Otherwise bs.raw with stringified JavaScript seems to be the only option 😢

@fakenickels
Copy link
Member

Hey there! Sorry about the delay, I was in vacation these past 2 weeks.
Everything is looking good to me! We just have a merge conflict there to solve looks like

(),
);

/* based on test, execute left or right */
Copy link
Member

Choose a reason for hiding this comment

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

left or right you mean the ternary logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My guess it was copypasted from reason-apollo examples, not sure what it means 😄
I've replaced it with the comment about link splitting from the official apollo docs.

@Yakimych
Copy link
Contributor Author

Yakimych commented Jan 7, 2020

@fakenickels No worries - thanks for reviewing. I've rebased the branch on master and resolved the conflicts. Nice to see that startPolling and stopPolling has landed in master!

) {
| (Some(prev), Some(newData)) =>
{
...prev, // NOTE: This only works with BuckleScript 7
Copy link
Member

Choose a reason for hiding this comment

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

it's a good demo of functionality with BS7!

];

// Defining those types and "%identity" converters below allows us to
// write the updateQuery in pure Reason and avoid bs.raw alltogether
Copy link
Member

Choose a reason for hiding this comment

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

maybe worths mentioning that we need to write these types by hand because we can't use right away those from graphql_ppx_re to not make people confused

gonna link this discussion

#65

@fakenickels fakenickels merged commit 4b28158 into reasonml-community:master Jan 8, 2020
@fakenickels
Copy link
Member

@all-contributors update @Yakimych to add code

@allcontributors
Copy link
Contributor

@fakenickels

I've put up a pull request to add @code! 🎉

@fakenickels
Copy link
Member

@all-contributors update @Yakimych for code

@allcontributors
Copy link
Contributor

@fakenickels

I could not determine your intention.

Basic usage: @all-contributors please add @Someone for code, doc and infra

For other usages see the documentation

@fakenickels
Copy link
Member

@all-contributors add @Yakimych for code, bugs

@allcontributors
Copy link
Contributor

@fakenickels

I've put up a pull request to add @Yakimych! 🎉

@fakenickels
Copy link
Member

As we are upgrading to BS7 I released this under a major version, now it is available as v5.0.0.

Thanks a lot @Yakimych !

@fakenickels
Copy link
Member

We had updated the lib itself but not the demo as you had noticed 🤔

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants