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

Fixing the navigation logic and Angular controllers for Middleware provider #708

Merged
merged 2 commits into from
Mar 16, 2017

Conversation

jkremser
Copy link
Member

@jkremser jkremser commented Mar 16, 2017

This is a followup on this PR #687

I have created another PR for tests #693 the tests can go later, this should be merged relatively soon, because it blocks couple of people.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1430831

@miq-bot add_label bug, middleware
@martinpovolny could you please review?

@miq-bot assign @martinpovolny

@miq-bot
Copy link
Member

miq-bot commented Mar 16, 2017

@Jiri-Kremser Cannot apply the following label because they are not recognized: technical bug

… and m. server group the own show partial is used (with angular controller initialization etc.)
…necessary now, when the own show logic is used.
@jkremser jkremser force-pushed the middleware-show-fix branch from 3206bac to b7bd3f7 Compare March 16, 2017 13:51
@martinpovolny
Copy link
Member

The loggic followed by the fix is correct for sure.

2 issues:

  1. rubocop and codeclimate are correct, please fix
  2. can you get @karelhala or someone else from your group to check this in the UI and ACK it?

@miq-bot
Copy link
Member

miq-bot commented Mar 16, 2017

Checked commits jkremser/manageiq-ui-classic@f295dfa~...b7bd3f7 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks good. 👍

@karelhala
Copy link
Contributor

Checked all screens which were not functional before this fix, everything seems to be working fine.
👍

@martinpovolny
Copy link
Member

martinpovolny commented Mar 16, 2017

Gentlemen, I accept your promise that there will be tests!

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.

4 participants