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

Follow-up for #2794 (fix the UrlGenerationError crash) #2909

Merged
merged 3 commits into from
Dec 5, 2017
Merged

Follow-up for #2794 (fix the UrlGenerationError crash) #2909

merged 3 commits into from
Dec 5, 2017

Conversation

AparnaKarve
Copy link
Contributor

Follow-up for #2794

@AparnaKarve AparnaKarve mentioned this pull request Dec 2, 2017
Merged
@AparnaKarve
Copy link
Contributor Author

@miq-bot add_label bug,gaprindashvili/yes

martinpovolny
martinpovolny suggested changes Dec 2, 2017
View reviewed changes
Copy link
Member

@martinpovolny martinpovolny left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The code looks good, including tests.

Please, fix the style issue and get someone test this in the UI.

Thanks!

@miq-bot
Copy link
Member

miq-bot commented Dec 4, 2017

Checked commits AparnaKarve/manageiq-ui-classic@1c73376~...9c14662 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🍰

@lfu
Copy link
Member

lfu commented Dec 5, 2017

Tested the fix.
After clicking on the associated service of a generic object instance, i was brought to the service object.
Quadicon hover url points to service object. 👍

@martinpovolny martinpovolny merged commit b9b7833 into ManageIQ:master Dec 5, 2017
@martinpovolny martinpovolny self-assigned this Dec 5, 2017
@martinpovolny martinpovolny added this to the Sprint 75 Ending Dec 11, 2017 milestone Dec 5, 2017
@martinpovolny
Copy link
Member

Thx, @lfu!

@simaishi
Copy link
Contributor

@AparnaKarve @martinpovolny This is a follow up for #2794 and that PR isn't marked as gaprindashvili/yes. Should PR 2794 be backported as well?

@AparnaKarve
Copy link
Contributor Author

@simaishi You can skip backporting #2794 to Gaprindashvili, if that is doable.

Consider this PR as a replacement for #2794.
Thanks.

@AparnaKarve AparnaKarve deleted the 2794_followup branch December 11, 2017 19:21
@simaishi
Copy link
Contributor

Without #2794, this PR will conflict backporting. I guess he question was really should both go, or both not go 😄

So I'll take 2794, then this PR to avoid conflict.

@AparnaKarve
Copy link
Contributor Author

So I'll take 2794, then this PR to avoid conflict.

Sounds good.

simaishi pushed a commit that referenced this pull request Dec 11, 2017
@martinpovolny @simaishi
Merge pull request #2909 from AparnaKarve/2794_followup
220038d 
Follow-up for #2794 (fix the UrlGenerationError crash)
(cherry picked from commit b9b7833)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 220038de0aec7f1907540bdc68161e1c118942cc
Author: Martin Povolny <[email protected]>
Date:   Tue Dec 5 19:53:51 2017 +0100

    Merge pull request #2909 from AparnaKarve/2794_followup
    
    Follow-up for #2794 (fix the UrlGenerationError crash)
    (cherry picked from commit b9b783314cc2908dba2f9ef3a749311072d26d69)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinpovolny martinpovolny martinpovolny approved these changes

Assignees

@martinpovolny martinpovolny

Labels
bug gaprindashvili/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AparnaKarve @miq-bot @lfu @martinpovolny @simaishi