search

contentful-labs / starter-gatsby-blog

Gatsby starter for a Contentful project from the community.
https://contentful.github.io/starter-gatsby-blog/
MIT License
195 stars 199 forks source link

Env readme update #149

Closed jpalmieri closed 3 years ago

jpalmieri commented 3 years ago

Addresses https://github.com/contentful/starter-gatsby-blog/issues/148

There may be a better way to do this 🤷

It's definitely awkward since there's (what seems to be) backwards compatibility for .contentful.json support. I'm assuming the intention is to get people to start using .env files, so I've updated the instructions and configuration sample files according to that assumption.

It's not clear to me why a production config file is also generated; maybe that could be clarified in the README and instructions.

axe312ger commented 3 years ago

@jpalmieri thank you!

We need dev and prod env files as Gatsby uses the one or the other depending on which command you use (gatsby develop or gatsby build) see https://github.com/contentful/starter-gatsby-blog/blob/master/gatsby-config.js#L2

Also: There will be a bug. The preview token will not work as the host is not passed to the plugin. see: https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-source-contentful#using-preview-api

axe312ger commented 3 years ago

And your assumptions are right: We should use .env files, these are the common way in Gatsby.

.contentful.json was never standardised

jpalmieri commented 3 years ago

@axe312ger What's the status on this? I'm not sure if your comments are feedback for this PR, or if they were just answering my question:

It's not clear to me why a production config file is also generated; maybe that could be clarified in the README and instructions.

Do I need to do anything else for this to be considered to be merged? Are there any problems with this PR? I've tested my changes by cloning my branch into a clean repo, running npm run setup, entering my API keys, and successfully starting the development server with npm run dev.

I'm not sure what the failure in the CI is about; I don't have enough context, but could take a look if someone could point me in the right direction. Thanks.

axe312ger commented 3 years ago

Yes, I agree with your changes.

About the merge.. I don't know. Just look at the other open PRs ;)

axe312ger commented 3 years ago

Like.. similar changes to yours tried to be merged in 2019: https://github.com/contentful/starter-gatsby-blog/pull/9

PR still open 🙃

jpalmieri commented 3 years ago

About the merge.. I don't know. Just look at the other open PRs ;) Like.. similar changes to yours tried to be merged in 2019: #9 PR still open 🙃

What do you mean by this? Are you saying that this repo is just not welcoming community contributions?

I assumed that was not the case, since the community version of the project (which I used to contribute to) was archived, with a note to use this project instead.

If no one cares about this PR, then I'm just going to close it. It seems to me like it's going to become stale like https://github.com/contentful/starter-gatsby-blog/pull/9 anyway.

✌️

axe312ger commented 3 years ago

PRs are welcome, but currently there is nobody here to merge them.