Skip to content

Conversation

@ilteoood
Copy link
Contributor

@ilteoood ilteoood commented Jan 4, 2025

Checklist

  • run npm run test and npm run benchmark

  • tests and/or benchmarks are included

  • documentation is changed or added

  • commit message and code follows the Developer's Certification of Origin
    and the Code of conduct

  • Migrated to node test runner

  • Removed simple-get in favour of fetch

@ilteoood ilteoood requested a review from dancastillo January 5, 2025 09:57
Co-authored-by: Dan Castillo <[email protected]>
Signed-off-by: Matteo Pietro Dazzi <[email protected]>
@ilteoood ilteoood requested a review from dancastillo January 5, 2025 15:39
Copy link
Member

@dancastillo dancastillo left a comment

Choose a reason for hiding this comment

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

Looks good, I would just updated the coverage check and add --check-coverage --lines 100 to test:unit and coverage to ensure it maintains coverage

ilteoood and others added 2 commits January 9, 2025 19:17
Co-authored-by: Dan Castillo <[email protected]>
Signed-off-by: Matteo Pietro Dazzi <[email protected]>
Co-authored-by: Dan Castillo <[email protected]>
Signed-off-by: Matteo Pietro Dazzi <[email protected]>
Copy link
Member

@dancastillo dancastillo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Fdawgs Fdawgs left a comment

Choose a reason for hiding this comment

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

c8 has gone missing from the dev dependencies, but is used in the coverage script.

@ilteoood ilteoood requested a review from Fdawgs January 10, 2025 11:55
@Fdawgs Fdawgs merged commit 6d1dcf8 into fastify:master Jan 10, 2025
8 of 11 checks passed
@Eomm Eomm mentioned this pull request Jan 18, 2025
4 tasks
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.

6 participants