Skip to content

Conversation

@aayushsanghavi
Copy link
Collaborator

  1. Look-wise the UI is decent. Functionality lacks some consistency. See screencasts below -
Screencast.from.09-24-2022.11.59.23.PM.webm
Screencast.from.09-25-2022.12.03.22.AM.webm
  1. Code quality is average. Please read the detailed comments provided below in the PR.
  2. Code lacks proper formatting and decreases readbility. Having a linter like eslint setup would be nice. See github actions - eslint-action-eslintrc or eslint-js-ts-action.
  3. Use style guides. All projects should follow the coding guidelines laid out for the technologies used. Google JS Styleguide, Google HTML and CSS styleguide and Airbnb React / JSX styleguide.
  4. Commit messages are poorly written. They indicate very little about the changes. Please read - Writing Good Commit Messages - A Practical Guide.
  5. For improving the serving of the backend look into a process manager like pm2. It offers a lot of really handy features that can improve the server startup, performance and process management.
  6. Handle errors properly. Just catching and logging them isn't enough. There should be a way to process the error. Good exception management is important for any app. Read about how to improve exception handling.
  7. Log at the right level with the right message in the right place. Proper logging is key to a good application.
  8. Code lacks tests. Even basic tests would be okay. Test driven development is a very standard and important industry practice. Read about it and try to add some basic form of testing to the code.
  9. Code lacks consistency -
    a. Some routes are defined in some index.js files but others are in the routes folder
    b. Some API functions are part of the controllers directory but others are defined in the routes and index files
    c. Some components have their dedicated folders with separate css and jsx files while others either have direct files in the outer folder or have all the styles defined inline with the jsx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants