-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: revamp the frontend architecture #6598
Conversation
contributes to - https://github.com/SigNoz/engineering-pod/issues/2068 |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
contributes to - #5621 |
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.
👍 Looks good to me! Incremental review on a95d218 in 13 minutes and 33 seconds
More details
- Looked at
109
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. frontend/src/container/LogsExplorerViews/index.tsx:292
- Draft comment:
Remove the unnecessaryconsole.log
statement to clean up the code. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. frontend/src/container/Login/index.tsx:86
- Draft comment:
Avoid using inline styles. Replace inline styles with CSS classes or styled components where applicable. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
3. frontend/src/container/Login/index.tsx:1
- Draft comment:
Avoid usingindex.tsx
for component files. Use a more descriptive file name for better debugging and searching. - Reason this comment was not posted:
Comment was on unchanged code.
4. frontend/src/container/LogsExplorerViews/index.tsx:292
- Draft comment:
Avoid usingindex.tsx
for component files. Use a more descriptive file name for better debugging and searching. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_tYinoYp2xQoGc5Vq
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
Minor Comments, can be taken up in a separate PR.
The base branch was changed.
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.
👍 Looks good to me! Incremental review on 420915e in 2 minutes and 8 seconds
More details
- Looked at
49
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/AppRoutes/Private.tsx:106
- Draft comment:
Avoid usingany
type forroute
parameter. Define a specific type forroute
to improve type safety and code readability. This applies tonavigateToWorkSpaceSuspended
as well. - Reason this comment was not posted:
Comment was on unchanged code.
2. frontend/src/AppRoutes/Private.tsx:123
- Draft comment:
Avoid using entire objects likemapRoutes
in dependency arrays ofuseEffect
. Use specific properties or values instead to prevent unnecessary re-renders. This applies to theuseEffect
on line 133 as well. - Reason this comment was not posted:
Comment was on unchanged code.
3. frontend/src/AppRoutes/Private.tsx:62
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_RSQj50oZqKn7eelE
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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 am approving to unblock.
Refactor
This PR takes care of moving away from redux as much as possible and streamlining the routing , sync setup calls and making sure we have deterministic actions based on API results. All the test cases have been fixed in sync with the latest design architecture
Contributes to - #5621 , https://github.com/SigNoz/engineering-pod/issues/2068