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

Refactoring of code base & developer documentation #62

Closed
aladwani opened this issue Dec 11, 2022 · 13 comments
Closed

Refactoring of code base & developer documentation #62

aladwani opened this issue Dec 11, 2022 · 13 comments
Assignees
Labels
v1 Issue needs to be resolved for first iteration

Comments

@aladwani
Copy link
Collaborator

Refactor the code of the app to make it more clear. Increase reusability and incode documentation (comments)

@aladwani aladwani added V0.9 Last version before upload Offer needed labels Dec 11, 2022
@aladwani aladwani changed the title Refactoring of code base Refactoring of code base & developer documentation Dec 11, 2022
@bodobraegger
Copy link
Collaborator

bodobraegger commented Dec 14, 2022

  • Try to remove unnecessary bloat, simplify layout for more consistent mobile views.
  • Simplify API where possible, remove unnecessary fields, move logic away from frontend where possible. I.e. Section#Chapter logic: just include proper parent pointers based on some unique identifier.
  • fix ignored URL prefixes
  • upgrade react to 17.X or 18.X, resolve dependencies accordingly
  • write dev guide after refactoring for (potentially new) dev and prod tooling

@aladwani
Copy link
Collaborator Author

I agree with the points mentioned above. Lets wait with the dev guide until the rest is done. There should also be a guide in the future how to set everything up in the background @jimmylevell.
I think phoenix mentioned to upgrade to ract 17.x should be enough for this sprint

@jimmylevell
Copy link
Collaborator

Totally agree on your listings and your insight. Would write out the dev and admin guide (#65) as soon as the next iteration (0.9) is completed as heavy code quality improvements and simplification are introduced.

I will created a task within the API project to address the admin / dev guide.

@jimmylevell
Copy link
Collaborator

jimmylevell commented Jan 3, 2023

Discussed in meeting

  • simplify URL (manageability within STRAPI) consider MUI and readable URLs
  • Dockerize everything so that it can be easier deployed Containerize Tech-Stack #80

bodobraegger added a commit that referenced this issue Feb 3, 2023
- remove unnecessary depdendencies
- separate production and dev dependencies
@bodobraegger
Copy link
Collaborator

Please, when adding new dependencies, think about whether they are dev or production dependencies and organize them accordingly.

@bodobraegger bodobraegger added v1 Issue needs to be resolved for first iteration and removed V0.9 Last version before upload Offer needed labels Feb 3, 2023
@jimmylevell
Copy link
Collaborator

@broesmel : could you point out which commit you mean where I have added dependencies to the wrong section? As I went through my latest commits and only saw that I have added "serve" cd45a63#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519.

Just that I can fix the wrong assignment. Thank you for pointing out! :)

@bodobraegger
Copy link
Collaborator

Hello! Sorry for the delay in answering.

Using "serve" as an example - which is not required in our production server as far as I know. When adding dependencies just used by developers, you can save them globally npm install -g <package_name> and leave them out of the project. Or, if you they are necessary for the development workflow, you can add them to the devDependencies npm install <package-name> --save-dev.
This avoids get compilation errors in case of dependency conflicts and so on.

Thanks!

bodobraegger added a commit that referenced this issue Apr 10, 2023
- replace with react-helmet async
- fixes untraceable console warnings
bodobraegger added a commit that referenced this issue Apr 10, 2023
bodobraegger added a commit that referenced this issue Apr 11, 2023
@bodobraegger
Copy link
Collaborator

bodobraegger commented Apr 11, 2023

@aladwani

Is the 'tasks' functionality as ported over the hering project relevant for thilo?

The same I ask myself for the 'targets', and most importantly for the 'links' which are fetched on every load.

edit after meet:
Functionality will be kept until initial content is filled and then we can check on how to maintain it.

@bodobraegger
Copy link
Collaborator

I consider this to be mainly done, now waiting for the content to see if everything holds up. The code has some comments now and a dev guide can be created once we are nearing release.

@jimmylevell
Copy link
Collaborator

Status of open tasks

  • Clear dev dependencies

@bodobraegger
Copy link
Collaborator

I think (most of) the dev dependencies need to remain in the project to facilitate development. They are never going into the production code so this should not be too big of an issue.

@jimmylevell
Copy link
Collaborator

@broesmel : ok so in this case the following point of yours has been resolved / addressed during your refactoring?

Please, when adding new dependencies, think about whether they are dev or production dependencies and organize them accordingly.

@bodobraegger
Copy link
Collaborator

Yes, exactly. Now, only production relevant dependencies are used in the build process, and developer dependencies are used during local development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1 Issue needs to be resolved for first iteration
Projects
None yet
Development

No branches or pull requests

3 participants