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

Add ADR 0009 for frontend directory structure #27

Merged
merged 1 commit into from
May 16, 2023

Conversation

dottorblaster
Copy link
Contributor

As above, just adding the accepted ADR for the new FE directory structure

Copy link
Member

@nelsonkopliku nelsonkopliku left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

It looks great @dottorblaster
A couple of comments and one question.
But I find it already mergable as well


There may be some overhead associated with restructuring the frontend, such as updating dependencies, fixing references, or rewriting some parts of the application. However, in the long term, this new directory structure will ensure an organized and efficient system that can be easily maintained, scaled, and navigated. The new structure will also facilitate collaboration between developers as they will have a better understanding of where everything is located.

We have one drawback: action creators related to sagas now live along with slices and action creators generated by redux-toolkit. We found reasonable to have action creators all stuffed in one specific place, but it is well understood that it might not be the case for everyone.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this exactly mean?
That the content we have now in actions folder goes to the reducers file?
Each of the action respectively with the domain file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, we can squash all together 👍


## Decision

We propose the following new directory structure for the frontend of our software project:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add some link to the react docs?
https://redux.js.org/faq/code-structure/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doin' this right now :D

```
assets/js
├── app.js
├── common
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be a good idea to define what this common holds.
According redux docs:

/common contains truly generic and reusable utilities and components.

Maybe we can put this sentence as paragraph below, so we have a written and aknowledged agreement hehe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you're totally right 👍

@dottorblaster
Copy link
Contributor Author

Just referencing trento-project/web#1355

Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

Hey thanks, LGTM :D

@dottorblaster dottorblaster merged commit 8dce22f into main May 16, 2023
@dottorblaster dottorblaster deleted the adr-fe-dir-structure branch May 16, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants