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

Renaming of provider_foreman files, code to configuration_manager #6756

Closed
wants to merge 5 commits into from
Closed

Renaming of provider_foreman files, code to configuration_manager #6756

wants to merge 5 commits into from

Conversation

h-kataria
Copy link
Contributor

  • Renamed views, controllers and other files to not be specific to provider_foreman
  • Renamed methods and actions, routes etc to use configuration_manager
  • Changed any links to point to configuration_manager instead of provider_foreman

follow up PR for #6730

Depends on core PR ManageIQ/manageiq#19949

@h-kataria
Renaming of provider_foreman files, code to configuration_manager
261e6ba 
- Renamed views, controllers and other files to not be specific to provider_foreman
- Renamed methods and actions, routes etc to use `configuration_manager`
- Changed any links to point to `configuration_manager` instead of `provider_foreman`

follow up PR for ManageIQ#6730
@h-kataria h-kataria force-pushed the provider_foreman_renaming branch from 8e9a3c3 to 261e6ba Compare March 11, 2020 14:02
@chessbyte chessbyte requested review from himdel and skateman March 11, 2020 18:21
@h-kataria h-kataria force-pushed the provider_foreman_renaming branch 2 times, most recently from cacc376 to ce2fcea Compare March 12, 2020 03:15
@h-kataria h-kataria force-pushed the provider_foreman_renaming branch from ce2fcea to 095bd67 Compare March 12, 2020 04:14
@h-kataria
Fixed broken spec tests
5068d3c
@h-kataria
Fixed code to get controller spec working
093efbe 
- Fixed issue introduced during renaming of modules in toolbar files that was caught by failure in controller spec test.
@h-kataria
addressed rubocop warning
852e5c3
@miq-bot
Copy link
Member

miq-bot commented Mar 12, 2020

Some comments on commits h-kataria/manageiq-ui-classic@261e6ba~...852e5c3

spec/controllers/configuration_manager_controller_spec.rb

  • ⚠️ - 495 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
  • ⚠️ - 503 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

@miq-bot
Copy link
Member

miq-bot commented Mar 12, 2020

Checked commits h-kataria/manageiq-ui-classic@261e6ba~...852e5c3 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.20.0, and yamllint
38 files checked, 4 offenses detected

app/presenters/menu/default_menu.rb

spec/controllers/configuration_manager_controller_spec.rb

@chessbyte
Copy link
Member

maybe @bdunne can run these changes against a Foreman/Satellite system to ensure it does not break the old flow.

@skateman
Copy link
Member

The change is totally okay from my side. However, I am a little worried about the amount of tech debt in the area. If we want to do future work here, we should probably consider increasing the consistency with the rest of the screens in order to avoid some pain in the future. cc @himdel

@h-kataria h-kataria removed the wip label Mar 12, 2020
@h-kataria h-kataria changed the title [WIP] - Renaming of provider_foreman files, code to configuration_manager Renaming of provider_foreman files, code to configuration_manager Mar 12, 2020
This was referenced Mar 20, 2020
Closed
Merged
@chessbyte chessbyte mentioned this pull request Mar 21, 2020
Closed
3 tasks
@h-kataria h-kataria closed this Mar 27, 2020
@h-kataria
Copy link
Contributor Author

Closed in favor of #6782

@h-kataria h-kataria deleted the provider_foreman_renaming branch May 13, 2020 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinpovolny martinpovolny Awaiting requested review from martinpovolny

@himdel himdel Awaiting requested review from himdel

@skateman skateman Awaiting requested review from skateman

Assignees
No one assigned
Labels
configuration management enhancement pending core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@h-kataria @miq-bot @chessbyte @skateman