Skip to content

feat: export RowDataPacket, ResultSetHeader, ProcedureCallPacket from MySQL2 #133

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
Merged
merged 7 commits into from
Apr 24, 2024
Merged

feat: export RowDataPacket, ResultSetHeader, ProcedureCallPacket from MySQL2 #133

merged 7 commits into from
Apr 24, 2024

Conversation

dancastillo
Copy link
Member

Checklist

fixes: #127
This PR exports RowDataPacket, ResultSetHeader, ProcedureCallPacket from MySQL2 with prefix MySQL on the types

Uzlopak
Uzlopak reviewed Dec 29, 2023
View reviewed changes
mcollina
mcollina approved these changes Dec 30, 2023
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@dancastillo dancastillo requested a review from Uzlopak December 30, 2023 18:14
@Fdawgs Fdawgs requested a review from a team February 10, 2024 11:19
@Uzlopak
Copy link
Contributor

Uzlopak commented Feb 10, 2024

Hmm. There is not difference between the Promise typeguards and the normal typeguards?!

Eomm
Eomm requested changes Feb 10, 2024
View reviewed changes
@Uzlopak
Copy link
Contributor

Uzlopak commented Feb 10, 2024

Please not merge yet. Please give me time till mondaY

gurgunday
gurgunday approved these changes Feb 10, 2024
View reviewed changes
Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

It looks good to me as well

@dancastillo
Copy link
Member Author

@Uzlopak i updated to distinguish between normal and promise typeguards

@dancastillo dancastillo requested a review from Eomm February 17, 2024 03:19
Eomm
Eomm requested changes Feb 26, 2024
View reviewed changes
Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

Missing docs

@dancastillo dancastillo requested a review from Eomm March 9, 2024 03:09
pioupia
pioupia reviewed Mar 28, 2024
View reviewed changes
Copy link

@pioupia pioupia left a comment

Choose a reason for hiding this comment

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

It could help to pass unit tests:

pioupia
pioupia reviewed Mar 28, 2024
View reviewed changes
Copy link

@pioupia pioupia left a comment

Choose a reason for hiding this comment

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

It might be worth testing all possible cases to make sure the return values are right all the time.

@dancastillo
Copy link
Member Author

@Eomm @Uzlopak address comments could you ptal

@dancastillo dancastillo force-pushed the export-mysql2-types branch from b41d2ab to b08b412 Compare March 29, 2024 00:56
Eomm
Eomm reviewed Mar 29, 2024
View reviewed changes
Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

I can't see the is* functions documentation. Is this wanted?

@dancastillo dancastillo requested a review from Eomm April 20, 2024 01:17
@mcollina
Copy link
Member

@Eomm ptal

mcollina
mcollina approved these changes Apr 24, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

still lgtm

@mcollina mcollina merged commit a8007bc into fastify:master Apr 24, 2024
@dancastillo dancastillo deleted the export-mysql2-types branch August 12, 2024 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@Eomm Eomm Eomm approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@gurgunday gurgunday gurgunday approved these changes

@Uzlopak Uzlopak Awaiting requested review from Uzlopak

+1 more reviewer

@pioupia pioupia pioupia left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Export the RowDataPacket, ResultSetHeader and ProcedureCallPacket from MySQL2
7 participants
@dancastillo @Uzlopak @mcollina @Eomm @marco-ippolito @pioupia @gurgunday