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

change aggregation mixin methods into virtual attributes #20149

Merged
merged 1 commit into from
May 22, 2020

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented May 9, 2020

Keenan got this virtual aggregate stuff working and we should use it.

@miq-bot assign @kbrock

@miq-bot miq-bot added the wip label May 9, 2020
@d-m-u d-m-u changed the title [WIP] cleanup the aggregation mixin [WIP] change aggregation mixin methods into virtual attributes May 9, 2020
@d-m-u d-m-u force-pushed the virtual_aggregate_things branch 9 times, most recently from bba3159 to 3aeef98 Compare May 10, 2020 03:53
@d-m-u d-m-u force-pushed the virtual_aggregate_things branch 12 times, most recently from e849e74 to b27fb43 Compare May 12, 2020 22:45
@d-m-u d-m-u force-pushed the virtual_aggregate_things branch from 4af871f to 3eb91c5 Compare May 22, 2020 16:23
Copy link
Member

@NickLaMuro NickLaMuro left a comment

Choose a reason for hiding this comment

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

Uh... some more spec suggestions...

🎹 💨
🙉 (ducks)

@d-m-u d-m-u force-pushed the virtual_aggregate_things branch from 3eb91c5 to 2f28af3 Compare May 22, 2020 16:49
@d-m-u d-m-u force-pushed the virtual_aggregate_things branch from 2f28af3 to 354b7ee Compare May 22, 2020 17:00
@miq-bot
Copy link
Member

miq-bot commented May 22, 2020

Checked commit d-m-u@354b7ee with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
16 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Member

@NickLaMuro NickLaMuro left a comment

Choose a reason for hiding this comment

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

I think @kbrock has covered anything remaining with the models.

I am much more happy with the specs now, thanks! And this PR is already drastically different from when I looked at it, so thanks for taking all the suggestions/changes. Looks good to me.

@kbrock kbrock merged commit cf75130 into ManageIQ:master May 22, 2020
@d-m-u d-m-u deleted the virtual_aggregate_things branch May 22, 2020 21:20
kbrock added a commit to kbrock/manageiq that referenced this pull request May 26, 2020
kbrock added a commit to kbrock/manageiq that referenced this pull request May 26, 2020
introduced in:
ManageIQ#20149

```
  1) EmsClusterController#show render listnav partial correctly for timeline page
     Failure/Error: sdate, edate = @tl_record.first_and_last_event(@tl_options.evt_type)

     NameError:
       undefined local variable or method `all_host_ids' for #<EmsCluster:0x000055e3fd7fb930>
       Did you mean?  all_relationship_ids
```
kbrock added a commit to kbrock/manageiq that referenced this pull request May 26, 2020
introduced in:
ManageIQ#20149

```
  1) EmsClusterController#show render listnav partial correctly for timeline page
     Failure/Error: sdate, edate = @tl_record.first_and_last_event(@tl_options.evt_type)

     NameError:
       undefined local variable or method `all_host_ids' for #<EmsCluster:0x000055e3fd7fb930>
       Did you mean?  all_relationship_ids
```
d-m-u added a commit to d-m-u/manageiq-ui-classic that referenced this pull request Jun 17, 2020
fixes `ManageIQ::Providers::Google::CloudManager Inst Including Associations (0.1ms - 1rows)
[----] F, [2020-06-17T13:58:28.726000 #394303:ae790] FATAL -- : Error caught: [NoMethodError] undefined method 'all_vms_and_templates'` from ManageIQ/manageiq#20284

@miq-bot add_label bug

broken in ManageIQ/manageiq#20149
d-m-u added a commit to d-m-u/manageiq-ui-classic that referenced this pull request Jun 17, 2020
fixes `ManageIQ::Providers::Google::CloudManager Inst Including Associations (0.1ms - 1rows)
[----] F, [2020-06-17T13:58:28.726000 #394303:ae790] FATAL -- : Error caught: [NoMethodError] undefined method 'all_vms_and_templates'` from ManageIQ/manageiq#20284

broken in ManageIQ/manageiq#20149
@chessbyte
Copy link
Member

@h-kataria why do we want this backported to Jansa? Seems like it can live on master and get burned in for the Kasparov release.

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.

7 participants