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.

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

feat: todo example #353

Closed
wants to merge 4 commits into from
Closed

feat: todo example #353

wants to merge 4 commits into from

Conversation

ethanve
Copy link
Contributor

@ethanve ethanve commented Jun 8, 2020

No description provided.

@ethanve ethanve requested review from jquense, itajaja and taion June 8, 2020 18:28
Copy link
Contributor

@taion taion left a comment

Choose a reason for hiding this comment

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

i mostly just skimmed 5451a28...feat/example#diff-75c62265150520cb1789948a67223782R20

it looks like you re-synced the schema to https://github.com/relayjs/relay-examples/blob/master/todo/data/schema/index.js , and otherwise the react code is different in the new subscription logic + the use of relay-network-layer?

@@ -0,0 +1,5 @@
node_modules
Copy link
Contributor

Choose a reason for hiding this comment

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

could maybe use the same one as in base. or even just have the base package handle linting here

@@ -0,0 +1,4 @@
.DS_Store
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe not needed? shouldn't the base gitignore already handle this stuff?


useEffect(() => {
// initialize subscriptions
const removeDisposable = removeTodoSubscription();
Copy link
Contributor

Choose a reason for hiding this comment

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

might be better to name this as requestRemoveTodoSubscription, &c.

@ethanve
Copy link
Contributor Author

ethanve commented Jun 9, 2020

Really just copy/pasted the relay todo example, diffed and copied our flow version, then just got it to work with relay-network-layer. So basically what you said 😆

@ethanve ethanve closed this May 20, 2021
@jquense
Copy link
Member

jquense commented May 20, 2021

I SEE YOU ETHAN

@jquense jquense deleted the feat/example branch May 20, 2021 14:59
@taion
Copy link
Contributor

taion commented May 20, 2021

Why didn't we just merge this again?

@jquense
Copy link
Member

jquense commented May 20, 2021

I don't know 👋 hey Jimmy

@jquense
Copy link
Member

jquense commented May 20, 2021

tbh i didn't realize this was here I would have used it in #407 where I added half of this

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.

3 participants