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.

Sign up for GitHub

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

Jump to bottom

Lint warnings for measurement panel #1837

Closed
joverlee521 opened this issue Aug 28, 2024 · 3 comments
Closed

Lint warnings for measurement panel #1837

joverlee521 opened this issue Aug 28, 2024 · 3 comments
Assignees
@joverlee521

Comments

@joverlee521
Copy link
Contributor

Changes in #1827 are raising lint warnings that I completely missed before merging.

From latest CI run on the default branch

Warning:    52:10  warning  React Hook useCallback has a missing dependency: 'fn'. Either include it or remove the dependency array     react-hooks/exhaustive-deps
Warning:   175:18  warning  React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead  react-hooks/exhaustive-deps
@joverlee521 joverlee521 self-assigned this Aug 28, 2024
@victorlin
Copy link
Member

Proposal to prevent this in the future: warnings should cause the lint job to fail (how and why)

joverlee521 added a commit that referenced this issue Aug 28, 2024
@joverlee521
measurements: Revert back to useMemo
344cd28 
Resolves lint warnings from #1837.

I wonder if I ran into lint warnings the first time I tried `useCallback`
which is why I ended up using `useMemo`? Noting in comments so I don't
flip-flop again.
joverlee521 added a commit that referenced this issue Aug 28, 2024
@joverlee521
measurements: Revert back to useMemo
f9c8038 
Resolves lint warnings from #1837.

I wonder if I ran into lint warnings the first time I tried `useCallback`
which is why I ended up using `useMemo`? Noting in comments so I don't
flip-flop again.
This was referenced Aug 28, 2024
Merged
Closed
@joverlee521
Copy link
Contributor Author

joverlee521 commented Aug 28, 2024 •
edited
Loading

Proposal to prevent this in the future: warnings should cause the lint job to fail (how and why)

Good proposal! We can continue conversation in #1839

@joverlee521
Copy link
Contributor Author

Resolved by #1838

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

@joverlee521 joverlee521

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@victorlin @joverlee521