open-telemetry / opentelemetry-js-contrib

OpenTelemetry instrumentation for JavaScript modules
https://opentelemetry.io
Apache License 2.0
707 stars 531 forks source link

Run prettier in the development workflow #2543

Open trivikr opened 4 days ago

trivikr commented 4 days ago

Is your feature request related to a problem? Please describe

Prettier was added in https://github.com/open-telemetry/opentelemetry-js-contrib/pull/1439, but I couldn't find where it's run in the CI. There don't seem to be IDE specific configurations or precommit hooks.

https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-js-contrib+prettier+NOT+path%3A*.md&type=code

Because of this, there are some files committed without prettier being run

$ npx prettier --write $(git ls-files '*.ts' '*.js')
...

$ git status --short | wc -l
     143

Describe the solution you'd like to see

Run prettier somewhere in the development cycle. A good place to start is GitHub CI.

Maybe add format and format:fix in root package.json similar to lint:markdown and lint:markdown:fix

    "format": "prettier --check $(git ls-files '*.ts' '*.js')",
    "format:fix": "prettier --write $(git ls-files '*.ts' '*.js')",

Describe alternatives you've considered

All packages already have lint and lint:fix scripts. Another option will be add equivalent format and format:fix which run prettier.

trivikr commented 4 days ago

I suggest formatting just the TypeScript files right now, as there are just 18 of them - and add CI check.

$ npx prettier --check $(git ls-files '*.ts')
Checking formatting...
[warn] examples/express/src/client.ts
[warn] examples/express/src/server.ts
[warn] examples/express/src/tracer.ts
[warn] examples/koa/src/client.ts
[warn] examples/koa/src/server.ts
[warn] examples/koa/src/tracer.ts
[warn] examples/mongodb/src/client.ts
[warn] examples/mongodb/src/server.ts
[warn] examples/mongodb/src/tracer.ts
[warn] examples/mongodb/src/utils.ts
[warn] examples/mysql/src/client.ts
[warn] examples/mysql/src/server.ts
[warn] examples/mysql/src/tracer.ts
[warn] examples/redis/src/client.ts
[warn] examples/redis/src/express-tracer-handlers.ts
[warn] examples/redis/src/server.ts
[warn] examples/redis/src/setup-redis.ts
[warn] examples/redis/src/tracer.ts
[warn] Code style issues found in 18 files. Forgot to run Prettier?

The CI check will fail if files are not formatted for future PRs. Documentation https://prettier.io/docs/en/cli.html#list-different

I can post a PR if maintainers give 👍

trivikr commented 3 days ago

I added npm scripts and workflow to run formatter in CI In https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2545

The formatting fixes will be added once an existing maintainers approves the workflow to run, and it fails as expected.