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: remove nyc package #4120

Merged
merged 5 commits into from
Jul 9, 2024
Merged

chore: remove nyc package #4120

merged 5 commits into from
Jul 9, 2024

Conversation

david-luna
Copy link
Member

Removes nyc since is not used anymore. Ref: #4067 (comment)

Checklist

  • Remove code
  • Add tests
  • Update TypeScript typings
  • Update documentation
  • Add CHANGELOG.asciidoc entry
  • Commit message follows commit guidelines

Copy link
Member Author

@david-luna david-luna left a comment

Choose a reason for hiding this comment

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

/test tav

@david-luna david-luna marked this pull request as ready for review July 4, 2024 10:17
@david-luna david-luna requested a review from trentm July 4, 2024 10:17
@david-luna
Copy link
Member Author

@trentm I've marked as ready for review but I have some questions I'd like to discuss

@@ -175,15 +175,6 @@ jobs:
- run: npm ls --all || true
- name: npm test
run: npm test
- uses: inception-health/otel-upload-test-artifact-action@v1
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for reviewer: removing this will make disappear the artefacts attached to the test actions. IIUC this transforms the tests results to an OTEL trace with data in JSON format for inspection but I don't recall using it.

Copy link
Member

Choose a reason for hiding this comment

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

This was added in #2805 as part of a change to support OTel-based tracing of our CI workflows. That tracing of CI workflows was dropped in #4024, but that PR forgot to remove this part.

@david-luna
Copy link
Member Author

@trentm as discussed I've removed report generation to test_output folder. Ready for 👀

@@ -175,15 +175,6 @@ jobs:
- run: npm ls --all || true
- name: npm test
run: npm test
- uses: inception-health/otel-upload-test-artifact-action@v1
Copy link
Member

Choose a reason for hiding this comment

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

This was added in #2805 as part of a change to support OTel-based tracing of our CI workflows. That tracing of CI workflows was dropped in #4024, but that PR forgot to remove this part.

@@ -7,9 +7,6 @@
/.pnp
.pnp.js

# testing
/coverage

Copy link
Member

Choose a reason for hiding this comment

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

nit: Fine to remove this, but it is unrelated to the coverage tooling in the top-level package.

@@ -192,7 +191,6 @@
"mysql2": "^3.2.4",
"ndjson": "^2.0.0",
"numeral": "^2.0.6",
"nyc": "^15.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Can also npm uninstall tap-junit.

Copy link
Member Author

Choose a reason for hiding this comment

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

done! thanks for pointing it out :)

@david-luna david-luna requested a review from trentm July 8, 2024 09:36
@david-luna david-luna merged commit 0a94abf into main Jul 9, 2024
20 checks passed
@david-luna david-luna deleted the dluna/drop-nyc branch July 9, 2024 08:29
trentm pushed a commit that referenced this pull request Jul 24, 2024
PeterEinberger pushed a commit to fpm-git/apm-agent-nodejs that referenced this pull request Aug 20, 2024
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.

2 participants