Skip to content

stub: mark CallStreamObserver stable #8484

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

Closed
wants to merge 1 commit into from

Conversation

benjaminp
Copy link
Contributor

I believe this was overlooked in #7938's work to fix #1788.

I believe this was overlooked in grpc#7938's work to fix grpc#1788.
@benjaminp benjaminp force-pushed the stablize-CallStreamObserver branch from 9455dec to 8b1f28e Compare September 7, 2021 17:58
@ejona86
Copy link
Member

ejona86 commented Sep 8, 2021

This class is not intended to be stable. See #1788 (comment) and https://github.com/grpc/grpc-java/pull/7938/files/b1ddff181ac068f805492b1f2a4fa4b64c2311f4#diff-603fa39ffdc9115328b9a77d2a10a27d5f5a75f77a22f3d552bdf2d7ee6bf208, both of which you referenced.

Note: the previous javadoc formatting wasn't wrong and didn't seem unreadable, so it is best left as-is. Different people have different views of "best" for things like that and making changes like that causes unhelpful "style fighting" where the style changes back-and-forth depending on who touched the file last.

While maybe #1788 was closed prematurely and could be reopened, so much of that has been stabilized I think it may be a better idea to open a new issue to track CallStreamObserver.

@benjaminp
Copy link
Contributor Author

This class is not intended to be stable. See #1788 (comment) and https://github.com/grpc/grpc-java/pull/7938/files/b1ddff181ac068f805492b1f2a4fa4b64c2311f4#diff-603fa39ffdc9115328b9a77d2a10a27d5f5a75f77a22f3d552bdf2d7ee6bf208, both of which you referenced.

Thanks for pointing me to those. I got confused seeing an unstable base class with its methods overridden by stable subclasses.

Note: the previous javadoc formatting wasn't wrong and didn't seem unreadable, so it is best left as-is. Different people have different views of "best" for things like that and making changes like that causes unhelpful "style fighting" where the style changes back-and-forth depending on who touched the file last.

Sorry, it was not my intention to start any sort of style war. This is just what google-java-format spat out when I touched the annotation.

While maybe #1788 was closed prematurely and could be reopened, so much of that has been stabilized I think it may be a better idea to open a new issue to track CallStreamObserver.

#8499

@benjaminp benjaminp closed this Sep 9, 2021
@benjaminp benjaminp deleted the stablize-CallStreamObserver branch September 9, 2021 00:14
@ejona86
Copy link
Member

ejona86 commented Sep 9, 2021

This is just what google-java-format spat out when I touched the annotation.

Makes sense. Auto-code-formatters also cause style wars, potentially at higher scale, even with different versions of themselves. I've been disappointed by the crop of style formatters trying to reproduce gofmt's success. They ignored the one important aspect that made gofmt successful: it didn't throw away the existing formatting and only reformatted what was out-of-style. Oh, well.

#8499

@benjaminp, would you mind changing the link in the ExperimentalApi annotation of CallStreamObserver to point to your new issue?

@benjaminp
Copy link
Contributor Author

@benjaminp, would you mind changing the link in the ExperimentalApi annotation of CallStreamObserver to point to your new issue?

Sure, see #8502.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue for (Call|ServerCall)StreamObserver being Experimental.
2 participants