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

Add unstructured data example #9408

Merged
merged 6 commits into from
Oct 27, 2018

Conversation

amberleyromo
Copy link
Contributor

This adds the unstructured data site example built by @jlengstorf into the examples dir in the gatsby repo.

Copy link
Contributor

@jlengstorf jlengstorf left a comment

Choose a reason for hiding this comment

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

@amberleyromo How should we handle the comparison branch here? Same idea (create a branch, link to diffs)? Something else?

@amberleyromo
Copy link
Contributor Author

For now I changed the README to link to the branch on your repo.

These examples are supposed to be very targeted -- I think it makes sense in the context of your example repo, but is probably too confusing in the context of the monorepo. What do you think about making the "graphql" branch of this its own example?

@amberleyromo
Copy link
Contributor Author

What about as a local plugin example?

@jlengstorf
Copy link
Contributor

Hmm... I feel like the comparison between these examples is actually the most critical part. Maybe @gatsbyjs/core can weigh in here? Do we have any prior work that's comparing two approaches in Gatsby? Any ideas on how best to show that off?

@amberleyromo
Copy link
Contributor Author

Making it two sites with mirrored content with different approaches could accomplish the same thing well. In the "branch" solution, we still have to message in the README to 1) look at the branch and 2) what it's accomplishing.

With a "local plugin" example, we could show the gatsby-layer example, and also have it be its own standalone example -- still supported by messaging in both READMEs (same method as currently)

@jlengstorf
Copy link
Contributor

@amberleyromo Okay, I'm sold. Sounds good!

@amberleyromo
Copy link
Contributor Author

@jlengstorf Added the other example and modified both README files 👍

@@ -0,0 +1,56 @@
const axios = require("axios")
Copy link
Contributor

Choose a reason for hiding this comment

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

Complete and total nit-pick, but did we change something where strings use double quotes or something now? 🤷‍♂️

Promise.all(
names.map(async name => {
const { data: pokemon } = await get(`/pokemon/${name}`)
const abilities = await Promise.all(
Copy link
Contributor

@DSchau DSchau Oct 26, 2018

Choose a reason for hiding this comment

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

Minor suggestion!

Suggested change
const abilities = await Promise.all(
const abilities = await Promise.all(
pokemon.abilities.map(({ ability: { name: abilityName } }) =>
get(`/ability/${abilityName}`)
.then(res => res.data)
)
)

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for adding this, it'll be helpful to point to this!

One comment... it's not abundantly clear to me that unstructured data means not using GraphQL, and I think that's what people are going to be searching for rather than unstructured data, but I think that's probably OK!

@geekysrm geekysrm merged commit 5379143 into gatsbyjs:master Oct 27, 2018
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
* add @jlengstorf unstructured data in official examples

* clarify other branch in README

* add local plugin example (from comparison branch of unstructured data example

* update local plugin example README to reference unstructured data example

* update unstructured data README, and rename local plugins example dir

* link out to blog post and doc page
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.

None yet

4 participants