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

Refs #32534 - dont use SmartProxyDynflowCore #86

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

ezr-ondrej
Copy link
Member

Remove usage of SmartProxyDynflowCore.

Comment on lines 21 to 23
def on_proxy?
defined?(SmartProxyDynflowCore::Callback)
defined?(Proxy::Dynflow::Callback)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

This should now always be true, the action cannot run anywhere else than on the smart proxy

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to remove that then and remove usage of it?
Or just return always true and have a followup?

Copy link
Member Author

Choose a reason for hiding this comment

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

I feel the latter is what we want, so I went with it, I'll try to come up with follow-up

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem to be used outside of this repository so I'd be fine with removing it along with its usage in this pr

Copy link
Contributor

Choose a reason for hiding this comment

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

Hah, github didn't refresh. Nevermind then, let's do it in a followup

Remove usage of SmartProxyDynflowCore.
Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

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

Works well, ACK

@adamruzicka adamruzicka merged commit ac8abb0 into theforeman:master Jun 23, 2021
@adamruzicka
Copy link
Contributor

Thank you @ezr-ondrej !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants