fix: correct AppendSerializtionError typo #2037

Neenu1995 · 2023-03-13T17:20:13Z

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 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.

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…gleapis/java-bigquerystorage into fix-appendSerializationError

Neenu1995 · 2023-03-13T18:43:42Z

cc: @suztomo
This is to fix the typo without a breaking change.

suztomo · 2023-03-14T02:13:41Z

"ci / clirr" passing is a nice.

suztomo

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?

suztomo · 2023-03-14T02:17:58Z

google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/Exceptions.java

@@ -244,6 +244,17 @@ public String getStreamName() {
    }
  }

+  public static class AppendSerializationError extends AppendSerializtionError {


Javadoc? (I expect a copy of AppendSerializtionError) Explanation of typo and backward compatibility is nice.

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.

That makes sense.

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…gleapis/java-bigquerystorage into fix-appendSerializationError

Neenu1995 · 2023-03-17T15:31:19Z

@suztomo Would you mind taking another look?
Can you also confirm that, the class with the wrong typo name can be removed during next major version update?

suztomo · 2023-03-17T16:01:35Z

Let's say you merge this. How would library users know they use AppendSerializationError (no typo)?

Neenu1995 · 2023-03-17T16:21:24Z

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.

Neenu1995 · 2023-03-20T20:50:08Z

@yirutang PTAL

suztomo

I also think this is the best under the restriction of no breaking changes.

google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/Exceptions.java

* 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>

fix: correct AppendSerializtionError typo

f05a58c

Neenu1995 requested review from a team and prash-mi March 13, 2023 17:20

product-auto-label bot added size: m Pull request size is medium. api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. labels Mar 13, 2023

Neenu1995 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Mar 13, 2023

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 13, 2023

🦉 Updates from OwlBot post-processor

5c1c803

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

gcf-owl-bot bot requested a review from a team as a code owner March 13, 2023 17:22

gcf-owl-bot bot added 2 commits March 13, 2023 17:23

🦉 Updates from OwlBot post-processor

6052442

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'fix-appendSerializationError' of https://github.com/goo…

b8940f8

…gleapis/java-bigquerystorage into fix-appendSerializationError

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 13, 2023

suztomo reviewed Mar 14, 2023

View reviewed changes

chore: add javadoc

c237a23

Neenu1995 added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 14, 2023

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 14, 2023

gcf-owl-bot bot added 3 commits March 14, 2023 18:53

🦉 Updates from OwlBot post-processor

ee17683

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

🦉 Updates from OwlBot post-processor

acb10b5

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'fix-appendSerializationError' of https://github.com/goo…

227c956

…gleapis/java-bigquerystorage into fix-appendSerializationError

Neenu1995 requested a review from suztomo March 15, 2023 20:20

Neenu1995 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 20, 2023

farhan0102 approved these changes Mar 20, 2023

View reviewed changes

Neenu1995 requested review from yirutang and removed request for prash-mi March 20, 2023 20:49

suztomo approved these changes Mar 21, 2023

View reviewed changes

yirutang reviewed Mar 21, 2023

View reviewed changes

google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/Exceptions.java Show resolved Hide resolved

Merge branch 'main' into fix-appendSerializationError

4cf3817

Neenu1995 requested a review from a team as a code owner March 22, 2023 15:25

Neenu1995 requested a review from yirutang March 22, 2023 15:25

Neenu1995 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 22, 2023

Neenu1995 merged commit e67e913 into main Mar 22, 2023
17 checks passed

Neenu1995 deleted the fix-appendSerializationError branch March 22, 2023 16:04

Neenu1995 mentioned this pull request Mar 22, 2023

fix!: correct AppendSerializtionError typo #1901

Closed

4 tasks

release-please bot mentioned this pull request Mar 30, 2023

chore(main): release 2.34.2 #2067

Merged

arturowczarek mentioned this pull request Apr 6, 2023

feat: add schema aware stream writer #2048

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correct AppendSerializtionError typo #2037

fix: correct AppendSerializtionError typo #2037

Neenu1995 commented Mar 13, 2023 •

edited

Neenu1995 commented Mar 13, 2023

suztomo commented Mar 14, 2023

suztomo left a comment •

edited

suztomo Mar 14, 2023

Neenu1995 Mar 14, 2023

suztomo Mar 17, 2023

Neenu1995 commented Mar 17, 2023

suztomo commented Mar 17, 2023

Neenu1995 commented Mar 17, 2023

Neenu1995 commented Mar 20, 2023

suztomo left a comment

fix: correct AppendSerializtionError typo #2037

fix: correct AppendSerializtionError typo #2037

Conversation

Neenu1995 commented Mar 13, 2023 • edited

Neenu1995 commented Mar 13, 2023

suztomo commented Mar 14, 2023

suztomo left a comment • edited

Choose a reason for hiding this comment

suztomo Mar 14, 2023

Choose a reason for hiding this comment

Neenu1995 Mar 14, 2023

Choose a reason for hiding this comment

suztomo Mar 17, 2023

Choose a reason for hiding this comment

Neenu1995 commented Mar 17, 2023

suztomo commented Mar 17, 2023

Neenu1995 commented Mar 17, 2023

Neenu1995 commented Mar 20, 2023

suztomo left a comment

Choose a reason for hiding this comment

Neenu1995 commented Mar 13, 2023 •

edited

suztomo left a comment •

edited