-
Notifications
You must be signed in to change notification settings - Fork 72
Add app description #208
Add app description #208
Conversation
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.
Text is good.
I would adjust the Model Mapping and Data Fetching for a smoother flow.
I would love to have in every section some code sample in pseudo typescript. I have drafted some, so try to apply this.
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 is already one section about deployment
- rut lint on readme
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
|
I have merged data fetching and content modelling into one section, however it seemed to me better to have only one code block there, as it does not make much sense to showcase how the interface looks like. On the other hand as you said people mostly find the the code and then read the lines above... Should I somehow add some generic interface for type there? From other changes, I made some paragraphs shorter, as I felt that some information were unnecessary. I removed some links, because I thought that it may annoy people more, when there is a lot of links to other files. Also I run prettier on the file. And lastly I have added code blocks for more paragraphs. For instance for Handling 404 I did not add any, it only explain concept what we did, not what developer can do in the project. |
Simply007
left a comment
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.
For further reference - the best form for readme is to use passive. If you are up for it - you can rewrite the text in this form, but let's keep it optional - for important parts - I have placed the suggestions.
Before sending to another review - run the text using Grammarly.
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Simply007
left a comment
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.
A couple of nitpicks
README.md
Outdated
| In Kontent each language is identified by codename, in case of this project, it is `en-US` and `es-ES`. | ||
| ### Resoure strings |
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.
| ### Resoure strings | |
| ### Resource strings | |
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Simply007
left a comment
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.
One last thing
Co-authored-by: Ondřej Chrastina <9218736+Simply007@users.noreply.github.com>
Motivation
Update readme to describe how the application works
Checklist
How to test
If manual testing is required, what are the steps?