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

Please auidit the code and make possible changes on codebase #1

Open
uwussimo opened this issue Apr 17, 2023 · 12 comments
Open

Please auidit the code and make possible changes on codebase #1

uwussimo opened this issue Apr 17, 2023 · 12 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed

Comments

@uwussimo
Copy link
Contributor

No description provided.

@uwussimo uwussimo added bug Something isn't working enhancement New feature or request help wanted Extra attention is needed labels Apr 17, 2023
@Prathapkarumanchi
Copy link

Can you seeing this error message?
Screenshot 2023-04-17 at 9 17 09 PM

@xelopsys
Copy link
Contributor

Screenshot 2023-04-18 at 11 58 24

why you are trying to fetch blogsList an object?

@xelopsys
Copy link
Contributor

Screenshot 2023-04-18 at 11 54 52

it's not necessary as long as you have [component] page so it catches all other routes, but if we consider developing it further it would be best to overwrite login page with all components from [component] and all other routes would 404* page

@xelopsys
Copy link
Contributor

Screenshot 2023-04-18 at 11 54 07

i recommend you to not define environment properties within constants, cause environment variables will be declared on build time, so it would be best just to declare it with process.env , not depending on any other constants

@shivamvora
Copy link
Collaborator

shivamvora commented Apr 18, 2023 via email

@xelopsys
Copy link
Contributor

yes it would be better

@shivamvora
Copy link
Collaborator

shivamvora commented Apr 18, 2023 via email

@xelopsys
Copy link
Contributor

Screenshot 2023-04-18 at 11 52 16

Screenshot 2023-04-18 at 11 52 22

and please stick to one type of declaring functional components in react, and it would be better if you use "function"

@xelopsys
Copy link
Contributor

and the rest looking good! Good job!

@shivamvora
Copy link
Collaborator

shivamvora commented Apr 18, 2023 via email

@xelopsys
Copy link
Contributor

Screenshot 2023-04-18 at 11 54 52

it's not necessary as long as you have [component] page so it catches all other routes, but if we consider developing it further it would be best to overwrite login page with all components from [component] and all other routes would 404* page

and one thing, dont use different languages at declaring pages, use typescript or javascript not both (it would be clean code).

@Prathapkarumanchi
Copy link

Screenshot 2023-04-18 at 12 42 23 PM

React component should start with Capital letter instead of blogDetail it should be BlogDetails
same with JoinList

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants