Skip to content

[Java Client] Fix JavaDoc for closeAsync: remove documented exception…#12005

Merged
eolivelli merged 1 commit intoapache:masterfrom
michaeljmarshall:fix-client-javadoc
Oct 4, 2021
Merged

[Java Client] Fix JavaDoc for closeAsync: remove documented exception…#12005
eolivelli merged 1 commit intoapache:masterfrom
michaeljmarshall:fix-client-javadoc

Conversation

@michaeljmarshall
Copy link
Member

… that isn't thrown

Motivation

Fix JavaDoc on the PulsarClient interface.

Changes

  • Switch throws to return as the method does not actually throw the checked exception

Testing

No tests needed, as this change just updates the documentation to align with the code.

Documentation

This is fixing documentation. The website does not need to be updated.

@michaeljmarshall
Copy link
Member Author

/pulsarbot run-failure-checks

1 similar comment
@michaeljmarshall
Copy link
Member Author

/pulsarbot run-failure-checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

We do not have a good way to document possibile error outcomes of CompletareFuture.
This is a general problem for libraries that use this mechanism.

I am not sure if we should document anyway the fact that some kind of error may occur

@Anonymitaet Anonymitaet added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Sep 13, 2021
@michaeljmarshall
Copy link
Member Author

Lgtm

We do not have a good way to document possibile error outcomes of CompletareFuture.
This is a general problem for libraries that use this mechanism.

I am not sure if we should document anyway the fact that some kind of error may occur

@eolivelli - this is a good point. We could choose to list some of the expected exceptions here. However, there won't be any compile time checks to ensure that implementations of this interface only throw documented exceptions.

Since this is a generic problem that extends even beyond Pulsar, I don't think we should try to solve it in this PR. I copied the updated comment from another client interface:

/**
* Asynchronously close the reader and stop the broker to push more messages.
*
* @return a future that can be used to track the completion of the operation
*/
CompletableFuture<Void> closeAsync();

@michaeljmarshall
Copy link
Member Author

@eolivelli eolivelli merged commit 6826434 into apache:master Oct 4, 2021
@eolivelli eolivelli added this to the 2.9.0 milestone Oct 4, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. release/2.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants