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[bmqa]: Remove unused Session::impl() function #519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pniedzielski
Copy link
Collaborator

This function is not used within the BlazingMQ code, and exposes publically the PIMPL implementation that is used by bmqa::Session. Because we do not use this function, and because it is part of the public (documented!) API, users could break our session invariants. This patch removes this function from the API. While this is a breaking change, we do not want any user to be using this function, so we should be okay with breaking them.

Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 375 of commit 7b73bc9 has completed with FAILURE

This function is not used within the BlazingMQ code, and exposes
publically the PIMPL implementation that is used by `bmqa::Session`.
Because we do not use this function, and because it is part of the
public (documented!) API, users could break our session invariants.
This patch removes this function from the API.  While this is a breaking
change, we do not want any user to be using this function, so we should
be okay with breaking them.

Signed-off-by: Patrick M. Niedzielski <[email protected]>
Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 376 of commit 9a0d2f5 has completed with FAILURE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Client Area: C++ SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants