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

Update README.md #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update README.md #6

wants to merge 1 commit into from

Conversation

MWest2020
Copy link
Member

API service chapter still needs to be added.

Please review the following updates for correct information.

Copy link

@lencodes lencodes left a comment

Choose a reason for hiding this comment

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

Looks good. I've been on vacation so I'm unsure what decisions have been made. I've left some suggestions and comments.

One general note: we have been working on a new skeleton application repository: skeleton-gatsby. As far as I'm aware, this repository has become deprecated because of that (and updating this documentation here would therefore redundant? the documentation itself is not redundant, but might be best moved to the new repository?)

README.md Show resolved Hide resolved
An ready out of the box basic application that doesn't require configuration or setup and can be extended immediately.

The application is available in two versions, a next.js version and an gatsby version. If you want to put your prototype online without setting up a server we recommend the gatsby version, since that can be deployed through github pages.
- Development and (online) demonstration of prototypes without needing a server.

Choose a reason for hiding this comment

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

Might be more accurate to say that when implementing, you do not need to create or maintain your own server; since a server is always needed when hosting something.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
You can edit these to define your own environment variables.
In the `pwa` folder there are 2 `.env` files used for local development and the static files.

These are called `.env.development` and `.env.production`. You can edit these to define your own environment variables.

Choose a reason for hiding this comment

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

This is something we have to update.

If we're going to add .env.development and .env.production into the repository (and therefore into git), Github will be able to see the changes. This can cause many issues (e.g. production keys being committed by accident).

A simple way to solve this is as followed:

Add .env.development and .env.production to the .gitignore and add two new files to the repository: .env.development.example and .env.production.example.

In these example files, we add all the KEYS (NOT the VALUES of these KEYS), like so (e.g.):

API_KEY_X=
OTHER_KEY=
YOU_GET_THE_IDEA=

In this documentation, we could explain that you have to copy these files, to the respective variants without .example and add they values to the required keys.

This way, when cloning a repository, you always know what keys you need to fill, but you can never push those keys to the repository.

Choose a reason for hiding this comment

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

Note: if we have values to keys that are publicly available, we could add these to the .example files I think.


When adding new variables make sure to also add them to the [urlContext](pwa/src/context/urlContext.tsx) file.
When adding new variables make sure to also add them to the `urlContext` file.

Choose a reason for hiding this comment

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

I don't think we want to keep doing 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.

2 participants