[EUWE] correct field names in reports #15498
Conversation
|
@miq-bot add_label wip |
miq-bot
changed the title
|
Checked commits jntullo/manageiq@0f977e1~...ced36f0 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
@miq-bot remove_label wip |
miq-bot
changed the title
|
Random comments from left field, I know! The specs could use some love IMO, specifically the Tag::REGEX. It's always good to ensure that the regex matches what you think it matches. I'm curious why it's Tag::REGEX but Field::FIELD_REGEX. Not a big deal, just wondering if there's some standard we should be adhering to. BTW, the current failure seems to be a Travis job issue, not an issue with the PR. |
| @@ -12,7 +12,7 @@ class MiqExpression::Field | |||
| ParseError = Class.new(StandardError) | |||
|
|
|||
| def self.parse(field) | |||
| match = FIELD_REGEX.match(field) or raise ParseError, field | |||
There was a problem hiding this comment.
Was this to fix a rubocop complaint? I wish it wouldn't complain about that! If it's not directly related, I would leave this change out
There was a problem hiding this comment.
Change LGTM. I'm not sure if it was impossible to git cherry-pick any of the upstream revisions so that it directly relates back to that (noting what conflicts you had) - that might have been better. But I'm good with this as far as the code is concerned. Thanks @jntullo ! 🎈
Euwe version of #14905
Needed to add a couple of missing elements (
REGEXandparse_params) to resolve conflicts and make this work. Tried to cherry pick as little as possible from the latest implementation ofMiqExpressionand only take what was necessary to get it to work as expected while fixing the BZ.@miq-bot add_label euwe/yes, bug
Screenshot of the report after this fix - properly colors the "Approved by" column
