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

[www/starters] tweaks to fix DX for no GH token on www #8718

Merged
merged 1 commit into from
Oct 2, 2018

Conversation

amberleyromo
Copy link
Contributor

@amberleyromo amberleyromo commented Oct 2, 2018

Relevant to #8700

The mocked defaults for when there's no token / no github data fetched were incorrectly formatted. This fix ensures that you can develop on www without a token.

cc @DSchau

@amberleyromo amberleyromo requested a review from a team as a code owner October 2, 2018 18:46
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.

Pulled this down locally, and it worked like I'd expect!

Left a few comments, but they're total nit picks and not substantive! Unless you want to "fix" them I'll merge when you're ready :)

allDependencies,
gatsbyDependencies: allDependencies
.filter(
([key, _]) => ![`gatsby-cli`, `gatsby-link`].includes(key) // remove stuff everyone has
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: but the second arg (_) isn't used here at all, right?

)
.filter(([key, _]) => key.includes(`gatsby`)),
miscDependencies: allDependencies.filter(
([key, _]) => !key.includes(`gatsby`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same in these other spots.

@@ -67,15 +67,7 @@ const StartersList = ({ urlState, starters, count, sortRecent }) => {
stars,
stub,
} = starter.fields.starterShowcase
const { url: demoUrl, repo: repoUrl } = starter
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Removed some client side code in favor of injecting it, correct? 🎉

@DSchau DSchau merged commit 7bfb4b0 into gatsbyjs:master Oct 2, 2018
lipis added a commit to lipis/gatsby that referenced this pull request Oct 3, 2018
* 'master' of github.com:gatsbyjs/gatsby: (72 commits)
  fix: fix wording in plopfile (gatsbyjs#8735)
  fix: navigateTo deprecation message (gatsbyjs#8745)
  Add deploying-to-heroku.md (gatsbyjs#8721)
  fix(docs): Link to instructions on adding to plugin library (gatsbyjs#8686)
  updated gatsby-plugin-remove-trailing-slashes docs to include link re… (gatsbyjs#8720)
  feat(www/starters): Add TypeScript and Contentful starter (gatsbyjs#8704)
  fix(docs): fix typos in template doc (gatsbyjs#8692)
  feat(www): change chevron direction on sticky responsive sidebar (gatsbyjs#8664)
  chore(release): Publish
  [integration] [cypress-gatsby] feat: Add Gatsby API support to our Cypress plugin (gatsbyjs#8641)
  chore(release): Publish
  fix(gatsby): add mjs config to webpack and resolve correctly (gatsbyjs#8717)
  feat(gatsby-plugin-netlify): add force option to createRedirect (gatsbyjs#8521)
  [www/starters] tweaks to fix DX for no GH token on www (gatsbyjs#8718)
  chore(release): Publish
  fix(gatsby-dev-cli): infer correct prefix from package path (gatsbyjs#8683)
  fix(www/starters): fix broken link to submission instructions (gatsbyjs#8715)
  chore(release): Publish
  feat(gatsby) : add createContentDigest helper (gatsbyjs#8687)
  feat: publish SendGrid case study blogpost (gatsbyjs#8592)
  ...
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