-
Notifications
You must be signed in to change notification settings - Fork 60
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
📝 Add doc for directory structure for coral. #93
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.
Couple of minor comments, but otherwise a solid proposal. Lets use this as a initial reference and adjust when we get busy with the implementation and face the real world.
│ ├── api | ||
│ ├── types | ||
│ └── index.ts | ||
└── servies/ |
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.
small typo: services
│ │ ├── LayoutForOneThing.test.tsx | ||
│ │ ├── ... | ||
│ │ ├── index.ts | ||
│ │ └── pages/ |
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 think we should move pages
on higher level on the app
root.
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.
Yes! That was a wrong indentation from me. Thanks for noticing!
|
||
Contains assets like scss and images. | ||
|
||
#### `components` (alternative `ui-elements`?) |
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.
Lets drop the alternative in parentheses. I think we should move forward with components
## 🗂️ Folder and file naming | ||
|
||
- folders are in `kebab-case` | ||
- TypeScript files are in `kebab-case` |
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 would use camelCase
with regular .ts
files. This is probably a personal preference, but what do you 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.
My personal preference is kebab-cases, but I'm fine with either 😅 linting will make sure everything is consistent either way (and consistency is the most important part here)!
I thought maybe we even go in using only kebab-cases (or snake_case) based on Git is case-sensitive and your filesystem may not be - Weird folder merging on Windows 🤔 This would make sure that we don't run into issues as described when contributor are using different machines. Directory and file ending would already convey the information what kind of code the file contains (react vs typescript). That's the only technical argument I found for file naming convention.
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.
Lets go forward with kebab-base
. Typescript also has a config to help prevent bugs for case insensitive systems [Ref].
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.
Using all kebab cases or only typescript files? In first case I would update the doc accordingly.
I think this is a great document for starting point. When we start the actual development work and we see something not working in the proposel mode, lets iterate on it. |
About this change - What it does
directory-structure.md
describing our planned approach to structure code for the ui-rewriteSolves #98