-
Notifications
You must be signed in to change notification settings - Fork 813
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
Fix lint warnings #1093
Comments
I see that most of them aren't really fixable (like use of |
Sounds reasonable to me. @dyladan WDYT? |
I think that's fine in many cases. A lot of these rules have configs that allow you to ignore cases where the linting rules commonly fail also so that might be worth looking into. |
The other option is to use the |
We can make eslint ignore specific rules as well. For instance, we can disable
This way we can still catch errors using the linter, even in the ignored section. |
Regarding unused vars: Actually, there are a lot of code which mocks real one, like NoopExporter and so on. These params are needed to not break consistency between components, https://eslint.org/docs/rules/no-unused-vars#argsignorepattern |
have time to fix that |
Should this issue remain open if the above prs fixed some lint warnings? I do see about 65 lint warning (as of today) when I run it in |
I think it is still valuable to fix the warnings. |
@dyladan 👋🏽 I am curious, will this issue remain forever-opened? |
Lint warnings are not emitted when running with lerna. You can run |
Thank you for that guidance! I look forward to helping to reduce those warnings :) |
Closing in favor of #4258 |
After #892 is merged, we see many warnings in the lint build. It would be nice if we can exclude or fix them.
The current lint logs looks very noisy and taking care of a new lint issue can be a daunting task in the future, especially for the new contributors.
Screenshot:
Link: https://app.circleci.com/pipelines/github/open-telemetry/opentelemetry-js/3706/workflows/82bf0bf2-c125-4663-a3b7-2396167d09cd/jobs/20435/parallel-runs/0/steps/0-105
/cc @OlivierAlbertini
The text was updated successfully, but these errors were encountered: