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

chore: add workflow to format using prettier #2545

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

trivikr
Copy link
Contributor

@trivikr trivikr commented Nov 20, 2024

Which problem is this PR solving?

Short description of the changes

  • In the first commit, I've added format command and run it in the workflow. Once an existing maintainer approves it, the CI will fail.
  • Once the CI is run, and fails, I'll update the PR to format the 18 files listed in Run prettier in the development workflow #2543 (comment), and the CI will be successful.

@trivikr trivikr requested a review from a team as a code owner November 20, 2024 18:01
@trivikr trivikr marked this pull request as draft November 20, 2024 18:04
@trivikr
Copy link
Contributor Author

trivikr commented Nov 20, 2024

Converted PR to draft as the formatting changes still need to be added before it can be merged.
A maintainer can approve the workflows though, while the PR is in draft.

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.75%. Comparing base (162feb1) to head (7b901a2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2545   +/-   ##
=======================================
  Coverage   90.75%   90.75%           
=======================================
  Files         169      169           
  Lines        8018     8018           
  Branches     1632     1632           
=======================================
  Hits         7277     7277           
  Misses        741      741           
---- 🚨 Try these New Features:

@trivikr
Copy link
Contributor Author

trivikr commented Nov 21, 2024

The "Format" workflow failed as expected

Run npm run format

> [email protected] format
> 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 1[8](https://github.com/open-telemetry/opentelemetry-js-contrib/actions/runs/11958404330/job/33337854936?pr=2545#step:5:9) files. Forgot to run Prettier?
Error: Process completed with exit code 1.

https://github.com/open-telemetry/opentelemetry-js-contrib/actions/runs/11958404330/job/33337854936?pr=2545

Comment on lines +24 to +25
Math.random().toString(36).substring(2, 15) +
Math.random().toString(36).substring(2, 15);

Check failure

Code scanning / CodeQL

Insecure randomness High

This uses a cryptographically insecure random number generated at
Math.random()
in a security context.
This uses a cryptographically insecure random number generated at
Math.random()
in a security context.
Copy link
Contributor Author

@trivikr trivikr Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix posted in #2546, as it's not related to formatting

@trivikr
Copy link
Contributor Author

trivikr commented Nov 21, 2024

Formatting was fixed in 7b901a2 (#2545) by running npm run format:fix

The "Format" CI is now successful.

@trivikr
Copy link
Contributor Author

trivikr commented Nov 21, 2024

The CodeQL error workflow was not there when the last time page was edited in #1938. The error is also in an example code.

It shouldn't be blocking this PR which just formats the code.

@trivikr trivikr marked this pull request as ready for review November 21, 2024 18:35
@trivikr
Copy link
Contributor Author

trivikr commented Nov 22, 2024

Marking as draft as CodeQL CI failure unrelated to formatting is fixed in #2546

This PR can be made ready once the fix is merged, and rebased.

@trivikr trivikr marked this pull request as draft November 22, 2024 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run prettier in the development workflow
1 participant