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

Move MiqExpression#parse to Target #22451

Closed
wants to merge 2 commits into from

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Apr 10, 2023

part of #22397

move MiqExpression.parse_field_or_tag => MiqExpression::Target#parse

  • Makes more logical sense to have as part of the Targets rather than generic MiqExpression.
  • Will convert the remaining parse_field_or_tag calls across in a followup. This PR got too big.

(weekend fun)

@kbrock
Copy link
Member Author

kbrock commented Apr 24, 2023

@Fryguy LMK if this is too big. It does have 2 commits to separate the moving parts.

Don't think we even Target.new, but want a base implementation in there
so we can use `&.column_type` over `try(:column_type)`

Target is an undetermined entity. We default to non-numeric/:string.
So even thought we could lookup the attribute type off of the target,
hard-coding to a string was more in line with the existing code.

will leverage column_type/numeric? in the next commit that converts
parse_field_or_tag => parse commit
They all change the same lines
Moving this over to MiqExpression so it can be used by other classes

previous commit already ensured column_type and numeric are part of the interface

leaving miq_expression's references to parse_field_or_tag alone because those will
be deleted once preprocess_exp! stores a tag reference

-

This is an obvious play towards converting the triple serialized parse statements into a single
And part of a play to drop get_col_info and only use Target.parse
@miq-bot
Copy link
Member

miq-bot commented May 15, 2023

Checked commits kbrock/manageiq@1532a01~...df0f147 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
10 files checked, 2 offenses detected

spec/lib/miq_expression/target_spec.rb

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.

None yet

3 participants