Skip to content
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

Run prettier in the development workflow #2543

Open
trivikr opened this issue Nov 19, 2024 · 2 comments
Open

Run prettier in the development workflow #2543

trivikr opened this issue Nov 19, 2024 · 2 comments

Comments

@trivikr
Copy link
Contributor

trivikr commented Nov 19, 2024

Is your feature request related to a problem? Please describe

Prettier was added in #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
Copy link
Contributor Author

trivikr commented Nov 19, 2024

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
Copy link
Contributor Author

trivikr commented Nov 20, 2024

I added npm scripts and workflow to run formatter in CI In #2545

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant