Conversation
Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/code/src/renderer/features/sidebar/components/UpdateBanner.tsx
Line: 7-17
Comment:
**Debug/temporary code left in production**
These lines were marked as `// TEMP` but are included in the PR and will ship to users. With `status` hardcoded to `"ready"`, `version` defaulting to `"v0.99.0"`, and `isEnabled` forced to `true`, every user will see the update banner on every app load regardless of actual update state. The `void realIsEnabled` line confirms the real value is never consulted.
This block must be removed before merging.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sidebar/stores/sidebarStore.ts
Line: 1
Comment:
**Store importing from a component**
`sidebarStore` imports `SIDEBAR_MIN_WIDTH` from a React component file. Stores importing from UI components creates an inverted dependency direction — components typically depend on stores, not vice versa. Consider moving the constant to a shared constants file (e.g. `src/renderer/constants/layout.ts`) so both the component and the store can import it without either depending on the other.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: increase sidebar min width" | Re-trigger Greptile |
| const realStatus = useUpdateStore((s) => s.status); | ||
| const realVersion = useUpdateStore((s) => s.version); | ||
| const realIsEnabled = useUpdateStore((s) => s.isEnabled); | ||
| const installUpdate = useUpdateStore((s) => s.installUpdate); | ||
|
|
||
| // TEMP: force-render "ready" banner for visual review of sidebar min-width | ||
| const status = "ready" as typeof realStatus; | ||
| const version = realVersion ?? "v0.99.0"; | ||
| const isEnabled = true; | ||
| void realIsEnabled; | ||
|
|
There was a problem hiding this comment.
Debug/temporary code left in production
These lines were marked as // TEMP but are included in the PR and will ship to users. With status hardcoded to "ready", version defaulting to "v0.99.0", and isEnabled forced to true, every user will see the update banner on every app load regardless of actual update state. The void realIsEnabled line confirms the real value is never consulted.
This block must be removed before merging.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sidebar/components/UpdateBanner.tsx
Line: 7-17
Comment:
**Debug/temporary code left in production**
These lines were marked as `// TEMP` but are included in the PR and will ship to users. With `status` hardcoded to `"ready"`, `version` defaulting to `"v0.99.0"`, and `isEnabled` forced to `true`, every user will see the update banner on every app load regardless of actual update state. The `void realIsEnabled` line confirms the real value is never consulted.
This block must be removed before merging.
How can I resolve this? If you propose a fix, please make it concise.| @@ -1,3 +1,4 @@ | |||
| import { SIDEBAR_MIN_WIDTH } from "@components/ResizableSidebar"; | |||
There was a problem hiding this comment.
Store importing from a component
sidebarStore imports SIDEBAR_MIN_WIDTH from a React component file. Stores importing from UI components creates an inverted dependency direction — components typically depend on stores, not vice versa. Consider moving the constant to a shared constants file (e.g. src/renderer/constants/layout.ts) so both the component and the store can import it without either depending on the other.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sidebar/stores/sidebarStore.ts
Line: 1
Comment:
**Store importing from a component**
`sidebarStore` imports `SIDEBAR_MIN_WIDTH` from a React component file. Stores importing from UI components creates an inverted dependency direction — components typically depend on stores, not vice versa. Consider moving the constant to a shared constants file (e.g. `src/renderer/constants/layout.ts`) so both the component and the store can import it without either depending on the other.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
b638b73 to
430e157
Compare
e9f285a to
9a2b540
Compare
430e157 to
f88cc92
Compare
9a2b540 to
143dec1
Compare
f88cc92 to
25b215d
Compare
143dec1 to
4fc6c84
Compare
25b215d to
8027df2
Compare
4fc6c84 to
935de6f
Compare
8027df2 to
6e52943
Compare
e2f8389 to
574aaf3
Compare
6e52943 to
1db72f2
Compare
574aaf3 to
5cc774e
Compare
1db72f2 to
79f4277
Compare
79f4277 to
b6575d8
Compare
Merge activity
|

Closes #1822