Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
base: master
Are you sure you want to change the base?
Update README.md #6
Changes from all commits
bbb4ef8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should have version numbers included I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docker has to be running to run these commands. I feel as if, if you're unfamiliar with Docker, this might not be clear at this point.
It might be good to add some info boxes to our documentation every now and then. To add some more context to why things might not yet be working. In this case, we could just add all the commands as already stated, but add an information box with e.g. "Make sure your docker is running on your local machine".
Example of how React does this in their documentation (yellow box):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commands for development and production differ, might be good to add the development ones here and add a production section of documentation in which we document the production commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be nice to be inside of one of those info boxes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this regarding the previous commands
docker-compose pull & up
? Or is the command missing here?"you can run the following command", but no command follows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has it been decided that we're going to use Den Haag's components? Last time I heard, we weren't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the hackathon they was a mention of using them and it that it would've been nice to add them. If not, I can replace that part
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if this is the way we want to implement this. Right now, we're just implementing the entire package like it's a javascript import (which might be the way to go, but I'm unsure).
We've done this in a different way in the
skeleton-gatsby
repository (the new skeleton application repository). You could check the/styling/index.css
file to see how we did it.I think that in both cases we're including all the design tokens right now, which we will have to refactor at some point in the future. Because right now (in this example) we're importing literally every design-token, to only make use of
--utrecht-color-blue-35
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are the lead, man ;)
Will implement all suggestions/
There was a problem hiding this comment.
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.):
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.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.