fix: correct AppendSerializtionError typo#2037
Conversation
|
cc: @suztomo |
|
"ci / clirr" passing is a nice. |
There was a problem hiding this comment.
I think there are 2 options: class AppendSerializationError extends AppendSerializtionError (super class has typo) or class AppendSerializtionError extends AppendSerializationError (base class has typo).
This pull request seems to have chosen the former. Would you write down the rationale?
| } | ||
| } | ||
|
|
||
| public static class AppendSerializationError extends AppendSerializtionError { |
There was a problem hiding this comment.
Javadoc? (I expect a copy of AppendSerializtionError) Explanation of typo and backward compatibility is nice.
There was a problem hiding this comment.
Since we don't want to break customers using the AppendSerializationError(with typo) and at the same time, move all the reference we have to that class in the library, to the class without typo, it made sense to use class AppendSerializationError extends AppendSerializtionError (super class has typo).
The other way round would mean, we move the code in AppendSerializtionError to AppendSerializationError for us to use in the library. That would break the external customers, if we don't move the code, that will cause code duplication.
|
@suztomo Would you mind taking another look? |
|
Let's say you merge this. How would library users know they use |
|
Customers who are not directly using this class will not be affected in any way. Any customer who is directly using the AppendSerializtionError(with typo) will continue to use it but should eventually move to AppendSerializationError(without typo), if we remove the class with typo. The purpose of doing this work around right now is to introduce the class without typo, without introducing a breaking change now because we don't want a major version update now. Eventually when we do a major version update, we should remove the class with typo. This will break customers who are directly using the class with typo. There is no way around it. For any users who may want to use the class from now, there is a warning on the class with typo that it will be removed. I could add a @deprecated annotation as well. |
|
@yirutang PTAL |
suztomo
left a comment
There was a problem hiding this comment.
I also think this is the best under the restriction of no breaking changes.
* fix: correct AppendSerializtionError typo * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: add javadoc * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [2.34.2](https://togithub.com/googleapis/java-bigquerystorage/compare/v2.34.1...v2.34.2) (2023-03-30) ### Bug Fixes * Correct AppendSerializtionError typo ([#2037](https://togithub.com/googleapis/java-bigquerystorage/issues/2037)) ([e67e913](https://togithub.com/googleapis/java-bigquerystorage/commit/e67e913f34fda4f4cc523c0248e5344232c0b736)) ### Dependencies * Update dependency com.google.cloud:google-cloud-bigquery to v2.24.0 ([#2054](https://togithub.com/googleapis/java-bigquerystorage/issues/2054)) ([e3156c7](https://togithub.com/googleapis/java-bigquerystorage/commit/e3156c7b525f7df2f3fe756f096e7fb1352fae8e)) * Update dependency com.google.cloud:google-cloud-bigquery to v2.24.1 ([#2056](https://togithub.com/googleapis/java-bigquerystorage/issues/2056)) ([a989ac6](https://togithub.com/googleapis/java-bigquerystorage/commit/a989ac63d813cc98dcc13200a950fe3edad10bdf)) * Update dependency com.google.cloud:google-cloud-bigquery to v2.24.3 ([#2058](https://togithub.com/googleapis/java-bigquerystorage/issues/2058)) ([9346667](https://togithub.com/googleapis/java-bigquerystorage/commit/934666737a92ec3220c6a186cc1af0f1adabb00c)) * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.6.0 ([#2063](https://togithub.com/googleapis/java-bigquerystorage/issues/2063)) ([965de7b](https://togithub.com/googleapis/java-bigquerystorage/commit/965de7bf78884cca30e6e6d672b74d734bda840d)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Since we don't want to break customers using the
AppendSerializtionError(with typo) and at the same time, move all the reference we have to that class in the library, to the class without typo, it made sense to use classAppendSerializationError extends AppendSerializtionError(super class has typo).The other way round would mean, we move the code in
AppendSerializtionErrortoAppendSerializationErrorfor us to use in the library. That would break the external customers, if we don't move the code, that will cause code duplication.