[dashboard] address lint warnings #8725
Conversation
| }; | ||
|
|
||
| // TODO: Both Experiment and Experiments are used for both code and typings here. |
|
|
||
| // TODO: This one smells like it could be some clever trickery, |
To be used for anchor tags that should've been buttons because they don't really bring the user to any new location.
trumbitta
force-pushed
the
trumbitta/dashboard-lint-warnings-3841
branch
from
8949a59 to
8700bf3
Compare
|
@easyCZ I see you're basically doing what I had done in #7757 (rejected because it was too much), resulting in the same problem (a lot of conflicts for pre-existing PRs). Now, no problem at all for the conflicts: I can just redo everything, but we need a strategy to avoid this from happening. I can do micro-PRs, 1 file per PR, but I think we need some sort of agreement that the file would get a fairly quick review or we will be at this for the rest of the year :D |
|
My latest proposal on how to avoid the merge conflicts and move forward https://discord.com/channels/816244985187008514/953232219261009950/953335514742788137 |
|
Following up from https://discord.com/channels/816244985187008514/953232219261009950/953335514742788137 I'll close this and I will start opening 1 PR per file 😊 |
Really appreciate these @trumbitta 🙏🏽 |
|
This is the first of the new smaller ones #8842 |
Description
The fixes are few and fairly easy so far, so I'm doing them all I guess.
Look at this beauty 💖
There are some TODOs I marked, which need clarifications/go-ahead by a couple Gitpodders:
I commented in the "code review" interface directly mentioning @geropl and @AlexTugarev.
There are a couple more TODOs that mark linter warnings I silenced, with code that would need a more complex refactor.
Do you want me to open issues for each of them?
@jankeromnes I was thinking we could also add a linting step to the CI (I'm assuming there's none, at least for the dashboard, at this time) so that the warnings "per se" don't become errors but the CI fails when there are linting warnings.
I'm not familiar with the system in use, but I can look into it if you want.
To complete this, we could then add the files that still need work to that
.eslintignoreyou were suggesting.Although I'd rather fix the linting warnings as soon as the problems arise (and fix everything in the dashboard right in this PR to start with a clean slate), because I fear files included in the
.eslintignorewould just become forgotten sooner than later 🙈 .Related Issue(s)
Fixes #3841
How to test
Use the dashboard 😬
Everything should still work, but warnings both in the terminal and in the browser devtools console should be gone for the files changed by this PR.
Release Notes
Documentation