-
Notifications
You must be signed in to change notification settings - Fork 585
chore(eslint): lint examples/bunyan and prepare for linting .js and .mjs files #2567
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
Conversation
…mjs files This adds support for 'npm run lint' in examples/bunyan, using the same base eslint.config.js used by most of the repo. There are ulterior motives: - This is a first step to linting all the examples using the same base config (rather than the spotty linting of examples using the separate "examples/.eslintrc". - The eslint.config.js changes support being used to lint .ts, .js, and .mjs files reasonably. I'll follow-up with a separate PR adding linting of .mjs files used for some ESM testing (e.g. "instrumentation-ioredis/test/fixtures/use-ioredis.mjs") Hattip to the following for some of the eslint config changes: https://github.com/open-telemetry/opentelemetry-js/pull/4446/files#diff-1a88e8e72db982c999acf146817a416a02d93547458e6525610cf69b24751af9
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2567 +/- ##
==========================================
+ Coverage 89.53% 89.76% +0.23%
==========================================
Files 182 187 +5
Lines 8837 9147 +310
Branches 1844 1884 +40
==========================================
+ Hits 7912 8211 +299
- Misses 925 936 +11 🚀 New features to boost your workflow:
|
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.
Should examples/bunyan
be added to the package.json#workspaces
list in order to run linter on CI regularly?
opentelemetry-js-contrib/package.json
Lines 79 to 86 in 8bfa21f
"workspaces": [ | |
"packages/*", | |
"plugins/node/*", | |
"plugins/web/*", | |
"propagators/*", | |
"detectors/node/*", | |
"metapackages/*" | |
] |
I was initially thinking no. Instead I was going to consider adding a |
I like this idea. It's super useful to have linters run against examples but shouldn't block any other builds. I would love to have proper linting on all the examples. |
That's right. Similarly to my comment before, while it's useful to have CI run on examples regularly, if they are not being kept up to date, we don't want to have problems with the packages themselves. We need to revisit our examples strategy in general though to ensure they are useful while also not hindering forward progress. |
This has a conflict but I think it can be merged after that |
…linting) In open-telemetry#2567 support was added to eslint.config.js for linting .js and .mjs files -- before it only supported .ts files. Originally this was added to support linting examples/... which includes many .js files. This change enables linting of .js and .mjs files in the rest of the repo. Refs: open-telemetry#2891
This adds support for 'npm run lint' in examples/bunyan, using the same
base eslint.config.js used by most of the repo.
There are ulterior motives:
base config (rather than the spotty linting of examples using the
separate "examples/.eslintrc".
.js, and .mjs files reasonably. I'll follow-up with a separate PR
adding linting of .mjs files used for some ESM testing. (coordinate linting of JS files in examples/* and .mjs files in the repo #2891 for the follow-up.)
(e.g. "instrumentation-ioredis/test/fixtures/use-ioredis.mjs")
other examples and get the top-level
npm run lint:examples
using those. (coordinate linting of JS files in examples/* and .mjs files in the repo #2891 for the follow-up.)
Hattip to the following for some of the eslint config changes:
https://github.com/open-telemetry/opentelemetry-js/pull/4446/files#diff-1a88e8e72db982c999acf146817a416a02d93547458e6525610cf69b24751af9
If it helps review: