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.

Sign up for GitHub

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

Jump to bottom

Remove qpid_client class and avoid resource defaults in qpid class #315

Merged
merged 2 commits into from Apr 8, 2021
Merged

Remove qpid_client class and avoid resource defaults in qpid class #315

merged 2 commits into from Apr 8, 2021

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Feb 11, 2021

No description provided.

@ehelms
Copy link
Member Author

ehelms commented Feb 11, 2021

The qpid_client class was targeted at supporting Pulp 2 use of Qpid.

@ehelms ehelms mentioned this pull request Feb 11, 2021
Merged
ekohl
ekohl reviewed Apr 7, 2021
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

For the changelog I think it would be clearer to write Remove cert::qpid_client class and the other commit Avoid resource defaults and set explicit parameters. Other than that this looks ok to me.

@ehelms
Copy link
Member Author

ehelms commented Apr 7, 2021

Updated to two commits

ekohl
ekohl reviewed Apr 7, 2021
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

It mentions qpid::client but really it removes certs::qpid_client.

@ehelms ehelms changed the title Clean up Qpid certificate deployment Remove qpid_client class and avoid resource defaults in qpid class Apr 7, 2021
wbclark
wbclark reviewed Apr 7, 2021
View reviewed changes
Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

The commit message (both short and full) still references qpid::client when it should be qpid_client or certs::qpid_client

LGTM otherwise

@ehelms
Remove qpid_client class
3ea34ec 
The qpid_client class was used to setup certificates for use
by Pulp 2 when communicating with Qpid. With Pulp 3 now being
the only supported version this set of certificates is no
longer needed.
@ehelms
Avoid resource defaults and set explicit parameters in qpid.pp
3e9d871
@ekohl ekohl merged commit 7152851 into theforeman:master Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wbclark wbclark wbclark left review comments

@ekohl ekohl ekohl approved these changes

Assignees
No one assigned
Labels
Backwards incompatible Needs testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ehelms @ekohl @wbclark @theforeman-bot