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(master): release 3.5.1 #2123

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jul 9, 2023

🤖 I have created a release beep boop

3.5.1 (2023-07-10)

Bug Fixes

  • improvements to allow to use Bun and tls (#2119) (fd44a2a)
  • missing ResultSetHeader[] to query and execute (f649486)

This PR was generated with Release Please. See documentation.

@github-actions github-actions bot force-pushed the release-please--branches--master--components--mysql2 branch from a0608ce to 09bfc25 Compare July 9, 2023 11:16
@wellwelwel
Copy link
Sponsor Collaborator

wellwelwel commented Jul 9, 2023

@sidorares, even using the prefix docs:, the release-please doesn't include it to changelog 😕

I tried to learn the release-please documentation, but found nothing related.

The only reference I could find about it was in a file from semver:
https://github.com/npm/node-semver/blob/main/release-please-config.json

@sidorares
Copy link
Owner

@sidorares, even using the prefix docs:, the release-please doesn't include it to changelog 😕

🤷 not sure how to help, we can ignore this issue for now and fix if/when we know a good solution

@wellwelwel
Copy link
Sponsor Collaborator

No problem, because the changedRows change, I will wait for a conclusion before to merge it 🙋🏻‍♂️

@sidorares
Copy link
Owner

sure. I'll think a bit more and come back to you tomorrow. I feel this feature might be not reliable and misleading and maybe should be marked as potentially deprecated in the future

@wellwelwel
Copy link
Sponsor Collaborator

sure. I'll think a bit more and come back to you tomorrow. I feel this feature might be not reliable and misleading and maybe should be marked as potentially deprecated in the future

Alright 🙋🏻‍♂️


For now 🚀

Screenshot 2023-07-09 at 10 56 46

@github-actions github-actions bot force-pushed the release-please--branches--master--components--mysql2 branch from 09bfc25 to 017aae8 Compare July 10, 2023 02:58
@sidorares
Copy link
Owner

@wellwelwel is there a way to deprecate a field in TS? Quick googling gives me jsdock comment option "@deprecated reason description" - is this the best option?

Reasons I want to remove completely changedRows:

  • it's confusing given affectedRows exist
  • not part of the protocol and is extracted from human readable message. Will break if wording is changed or server is localised or non-mysql server ( MariaDB etc ) choses to use different message.

Re 0 vs undefined when message can't be parsed - probably not that important right now

@wellwelwel
Copy link
Sponsor Collaborator

Yes, I also suggested that this be done in the OkPacket types of query and execute.
We can also add a message explaining the reason for this.

A simple example:

/** @deprecated */
UTF8_GENERAL50_CI: number;

@wellwelwel
Copy link
Sponsor Collaborator

If you think it's a good idea, we can do it before to release this version.

@sidorares
Copy link
Owner

If you think it's a good idea, we can do it before to release this version.

yes, let's do as part of this release in a small new PR

@wellwelwel
Copy link
Sponsor Collaborator

wellwelwel commented Jul 10, 2023

I soon do that, just finishing a task 🙋🏻‍♂️

In changedRows, do you have a preference about how to report this in addition to "deprecated"?

Otherwise, I can resume it from:

  • it's confusing given affectedRows exist
  • not part of the protocol and is extracted from human readable message. Will break if wording is changed or server is localised or non-mysql server ( MariaDB etc ) choses to use different message.

@sidorares
Copy link
Owner

"changedRows is deprecated and might be removed in the future major release. Please use affectedRows property instead"

@wellwelwel wellwelwel merged commit 9051caf into master Jul 10, 2023
@github-actions
Copy link
Contributor Author

@sidorares sidorares deleted the release-please--branches--master--components--mysql2 branch July 10, 2023 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants