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.

Sign up for GitHub

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

Jump to bottom

Run prettier in the development workflow #2543

Open
trivikr opened this issue Nov 19, 2024 · 2 comments · May be fixed by #2545
Open

Run prettier in the development workflow #2543

trivikr opened this issue Nov 19, 2024 · 2 comments · May be fixed by #2545
Labels
feature-request

Comments

@trivikr
Copy link
Contributor

trivikr commented Nov 19, 2024 •
edited
Loading

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 trivikr linked a pull request Nov 20, 2024 that will close this issue
Draft
@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
Assignees
No one assigned
Labels
feature-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: add workflow to format using prettier trivikr/opentelemetry-js-contrib
1 participant
@trivikr