asyncer-io / r2dbc-mysql

Reactive Relational Database Connectivity for MySQL. The official successor to mirromutth/r2dbc-mysql(dev.miku:r2dbc-mysql).
https://r2dbc.io
Apache License 2.0
181 stars 18 forks source link

Extract API to api package #253

Closed mirromutth closed 4 months ago

mirromutth commented 4 months ago

Motivation:

Extract public API to api package, see also #251

Modification:

Most of changes are to use strongly typed interfaces in the new api package instead of using r2dbc-spi interface directly. So, the modification seems large, but it is mainly changes such as implements ColumnMetadata -> implements MySqlColumnMetadata.

Result:

Standardized our public interfaces

mirromutth commented 4 months ago

Oh, You're right. since this is not backward compatible(possible breaking compatibility). I think we need to increment major or minor. WDYT?

@jchrys

Those users who will not be affected by this modification:

This modification will cause a compilation warning for those users:

This modification will cause those users who need to modify the code to:

I have 3 suggestions:

jchrys commented 4 months ago

Oh, You're right. since this is not backward compatible(possible breaking compatibility). I think we need to increment major or minor. WDYT?

@jchrys

Those users who will not be affected by this modification:

  • Use this driver with spi ConnectionFactories
  • Only used MySqlConnectionConfiguration to configure somethings, and did not involve CodecRegistrar

This modification will cause a compilation warning for those users:

  • User is using MySqlTransactionDefinition and ConstantSnapshotEngine to use release-specific transaction properties. They should see @Deprecated

This modification will cause those users who need to modify the code to:

  • User is using CodecRegistrar to extend custom Codec. They should use MySqlReadableMetadata instead of the old MySqlColumnMetadata as any Codec should support OutParameters, which is a necessary change for the purpose of complying with the r2dbc spec
  • User is using old MySqlConnection, MySqlRow, MySqlStatement, etc. Their code may need to be changed to api.MySqlXxx, which makes sense because it was unclear which parts of the original API were public and which parts were not. Now with the api package, this will not be the case

I have 3 suggestions:

  • Consider a cumulative update to 1.2.0, which will provide clearly needed new features and fix all currently known issues. This should require more work
  • Just use 1.1.3, it is the first version for public API, and CodecRegitrar changes is necessary for supporting OUT parameters in r2dbc spec
  • Consider skipping some patch versions, e.g. 1.1.10 is next

@mirromutth It appears that we have yet to adopt a unified approach to versioning. It would be advantageous for us to agree on a versioning strategy. For example, whether to follow Strict SemVer (https://semver.org/) or a more relaxed version of SemVer, etc. In my opinion, I would like to suggest a slightly relaxed form of SemVer. For backward incompatible changes or new features, we could update the minor version. For backward compatible changes or new features, we could update the patch version. And for major versions, we could reserve those for significant or 'kaboom' changes. In this case, I would like to suggest updating to version 1.2.0.

mirromutth commented 4 months ago

@jchrys Thanks.

I consider avoiding the version growing too fast, because actually this PR did not introduce much new content.

Maybe we should delay merging this PR until current known bugs are resolved. Then we consider upgrading to 1.2? WDYT?

jchrys commented 4 months ago

@mirromutth Thanks for sharing your thought. In that case, proceeding with the originally planned version 1.1.3 seems appropriate.

Could you please confirm if the following version plan aligns with what you have in mind? if right we can follow that rule.

Major: Increment for big bang changes. Minor: Increment when resolved all known bugs or for relatively big changes. Patch: increment for everything else.

mirromutth commented 4 months ago

LGTM, the major version increment of r2dbc-spi is also a big change

jchrys commented 4 months ago

LGTM, the major version increment of r2dbc-spi is also a big change

Got it. I agree.