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

fix: surface storage interface expectations correctly. #241

Merged
merged 2 commits into from Apr 14, 2020

Conversation

frankyn
Copy link
Member

@frankyn frankyn commented Apr 13, 2020

cc: @dmitry-fa , @athakor based on pending PRs.

We will continue to update the Storage interface and mark it as InternalExtensionOnly.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 13, 2020
@frankyn frankyn changed the title Surface Storage interface expectations correctly. fix: surface storage interface expectations correctly. Apr 13, 2020
@codecov
Copy link

codecov bot commented Apr 13, 2020

Codecov Report

Merging #241 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #241   +/-   ##
=========================================
  Coverage     63.50%   63.50%           
  Complexity      540      540           
=========================================
  Files            30       30           
  Lines          4762     4762           
  Branches        427      427           
=========================================
  Hits           3024     3024           
  Misses         1578     1578           
  Partials        160      160           
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/com/google/cloud/storage/Storage.java 80.57% <ø> (ø) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15cb267...423a4f4. Read the comment docs.

@dmitry-fa
Copy link
Contributor

This fix is a good solution for the situation with extending the Storage interface. My question, is this change allowed?
Quoting: InternalExtensionOnly

Adding this annotation to an API is considered API-breaking.

@frankyn
Copy link
Member Author

frankyn commented Apr 14, 2020

Hi @dmitry-fa, +1 to being allowed. I went through internal review/discussion with folks and it was a trade-off decision at this point. The annotation is only documentation and doesn't change how the interface can be used.

Also acknowledging that this library doesn't follow Semantic versioning strictly but looking for input from @JesseLovelace and @crwilcox on this point. I may end up removing it until later.

@frankyn frankyn merged commit 130a641 into master Apr 14, 2020
@frankyn frankyn deleted the internal-only-update branch April 14, 2020 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants