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

[WIP] Flatten worker settings #20975

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

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Jan 20, 2021

This PR flattens the complicated nesting of the worker settings in order to make it easier for end users to find what they are looking for. In particular, since YAML is whitespace delimited, it can be very easy to mess up the yaml nesting, and this should fix that problem. One downside of going from a tree structure to a flat structure is that you lose context of what is inherited. As such, this PR introduces a new key in the settings, :inherited_from which acts more like a comment to the editor and cannot be modified by the end user. However, with that information, they should be able to easily traverse up the inheritance chain to determine the values.

cc @jrafanie

WIP because

@chessbyte
Copy link
Member

good simplification!

@Fryguy Fryguy force-pushed the flatten_worker_settings branch from 98f5476 to 1e8242f Compare January 21, 2021 15:07
@Fryguy Fryguy force-pushed the flatten_worker_settings branch from 1e8242f to fdf09ab Compare January 22, 2021 22:58
@Fryguy Fryguy added this to the Lasker milestone Feb 2, 2021
miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Feb 5, 2021
Fryguy pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Feb 9, 2021
@Fryguy Fryguy force-pushed the flatten_worker_settings branch from 3985502 to 13044af Compare February 9, 2021 21:39
@Fryguy Fryguy force-pushed the flatten_worker_settings branch from 13044af to d166e42 Compare February 15, 2021 21:29
@miq-bot
Copy link
Member

miq-bot commented Feb 15, 2021

Checked commit Fryguy@d166e42 with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
7 files checked, 2 offenses detected

app/models/miq_worker.rb

  • ❗ - Line 199, Col 14 - Style/HashEachMethods - Use each_key instead of keys.each.
  • ❗ - Line 204, Col 13 - Performance/RegexpMatch - Use match? instead of =~ when MatchData is not used.

@chessbyte chessbyte removed this from the Lasker milestone Feb 16, 2021
@Fryguy Fryguy mentioned this pull request Feb 17, 2021
48 tasks
@chessbyte chessbyte added this to the Morphy milestone Feb 23, 2021
@miq-bot
Copy link
Member

miq-bot commented Mar 19, 2021

This pull request is not mergeable. Please rebase and repush.

@kbrock
Copy link
Member

kbrock commented Jul 22, 2021

Do we want to still move forward with this?
Or are these changes just too much for us at this time?

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Jul 22, 2021
@Fryguy
Copy link
Member Author

Fryguy commented Jul 23, 2021

I still want to move forward with this, but need to rework it, possibly to remove the inheritance entirely. I had spoken to @jrafanie a while back and didn't want to step on the work he was doing to fix the UI form. In that thread we discussed possibly dropping the inheritance entirely, which would mean this PR would stay, but it would be even less complicated.

See ManageIQ/manageiq-ui-classic#7725 (comment)

@chessbyte chessbyte removed this from the Morphy milestone Sep 15, 2021
@miq-bot miq-bot added the stale label Feb 27, 2023
@miq-bot miq-bot closed this Feb 27, 2023
@miq-bot
Copy link
Member

miq-bot commented Feb 27, 2023

This pull request has been automatically closed because it has not been updated for at least 3 months.

Feel free to reopen this pull request if these changes are still valid.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

@Fryguy Fryguy removed the stale label Jul 27, 2023
@Fryguy Fryguy reopened this Jul 27, 2023
@Fryguy Fryguy added the pinned label Jul 27, 2023
miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Jul 27, 2023
@miq-bot
Copy link
Member

miq-bot commented Jul 28, 2023

This pull request is not mergeable. Please rebase and repush.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do
Development

Successfully merging this pull request may close these issues.

6 participants