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 #34758: Match rename of remote_execution::ssh to remote_executio… #759

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Apr 11, 2022

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.

I'd prefer if you all have the same timestamps so it's easy to correlate. The easiest way to achieve this is to call ./bin/create-migration with all scenarios it applies to.

Comment on lines 2 to 3
answers['foreman_proxy::plugin::remote_execution::script'] = answers['foreman_proxy::plugin::remote_execution::ssh']
answers.delete('foreman_proxy::plugin::remote_execution::ssh')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
answers['foreman_proxy::plugin::remote_execution::script'] = answers['foreman_proxy::plugin::remote_execution::ssh']
answers.delete('foreman_proxy::plugin::remote_execution::ssh')
answers['foreman_proxy::plugin::remote_execution::script'] = answers.delete('foreman_proxy::plugin::remote_execution::ssh')

@adamruzicka
Copy link
Contributor

Also inconsistent dashes vs underscores in migration file names?

@ekohl
Copy link
Member

ekohl commented Apr 11, 2022

Sadly that was already the case before and now we at least (try) to keep them consistent per scenario. In #736 I tried to align them but didn't get around to finishing it.

@ehelms
Copy link
Member Author

ehelms commented Apr 20, 2022

[test foreman-installer]

@ehelms ehelms requested a review from evgeni April 20, 2022 23:58
@ehelms
Copy link
Member Author

ehelms commented Apr 20, 2022

@evgeni This is also ready now

@ehelms ehelms merged commit de6ca1e into theforeman:develop Apr 21, 2022
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.

5 participants