Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
new-log-viewer: Finish support for filtering by log-level and associated refactoring. #89
new-log-viewer: Finish support for filtering by log-level and associated refactoring. #89
Changes from 10 commits
9f002a8
f66481f
8edcbf6
275edc0
1accf00
75b5dec
4d79444
e6fc92f
5915bd1
a1983d5
e3b2b36
03f1670
e904a28
b112382
e7ad6eb
c13df92
2cc0803
04b2515
a420d7e
914a188
45c2e42
aec63d5
934c7c6
e4e4aee
26b0bf3
6528c56
e53a904
3016d14
93a46f9
807066b
d03da58
dd9c92e
a87a9fc
a4f7a0f
b1e26e2
ae97ab3
bdfd2e0
80ccfec
963cbd5
0808a7b
6551edf
217693a
6488bf8
edbf7e1
eb706d3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Shall we group these with the other useEffects?
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.
Remove unused import 'Nullable'
The import
Nullable
from"../../typings/common"
is not used in this file and can be safely removed to clean up the code.🧰 Tools
🪛 GitHub Check: lint-check
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.
Remove unused imports 'getMatchingLogEventNum' and 'getNewNumPages'
The functions
getMatchingLogEventNum
andgetNewNumPages
are imported but not used in this file. Please remove these unused imports to avoid confusion and potential issues.🧰 Tools
🪛 GitHub Check: lint-check
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.
🛠️ Refactor suggestion
Consider enhancing error message for clarity.
When throwing an error due to a failure in setting the log level filter, including the attempted
logLevelFilter
value can provide more context for debugging.Apply this change to improve the error message:
📝 Committable suggestion
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 agree with the rabbit on this.
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.
@junhaoliao, glad to hear that! (^.^)/
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.
Shall we name
this.#numEvents
asthis.#numTotalEvents
?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'm okay to do this. shall we rename in front-end as well? In front-end, it is always the total.
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.
Yeah, sounds like a good idea.
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.
for now I just changed to activeNumEvents in this function. There are many many instances of numEvents in the code.