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

Add shared processor pool field option to provision form #8522

Merged

Conversation

alizapeikes
Copy link
Contributor

@alizapeikes alizapeikes commented Nov 14, 2022

@miq-bot
Copy link
Member

miq-bot commented Nov 14, 2022

Checked commit alizapeikes@ac6d021 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 10 offenses detected

app/views/shared/views/_prov_dialog.html.haml

  • ⚠️ - Line 433 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 433 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 433 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 433 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 433 - Layout/ExtraSpacing: Unnecessary spacing detected.
  • ⚠️ - Line 433 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 433 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 433 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 433 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.
  • ⚠️ - Line 433 - Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.

@jeffibm
Copy link
Member

jeffibm commented Nov 15, 2022

@alizapeikes , could you please add few screenshots and fix the lint errors...

@alizapeikes
Copy link
Contributor Author

@alizapeikes , could you please add few screenshots and fix the lint errors...

@jeffibm I am not sure why I am getting those lint errors, I set it up exactly the same way as the code before and after?

@jeffibm
Copy link
Member

jeffibm commented Nov 17, 2022

@alizapeikes , could you please add few screenshots and fix the lint errors...

@jeffibm I am not sure why I am getting those lint errors, I set it up exactly the same way as the code before and after?

@alizapeikes, Yes. we can fix lint errors later as the lines are aligned with the previous line of codes.

@miyamotoh
Copy link
Contributor

@jeffibm @agrare May I know what's holding up this PR? I tested this locally, and UI didn't break. I then applied the related PR for PowerVS also, and the depicted option showed up and got the new VM on the pool as intended.

I can follow up with the lint error, but it's actually all over the .haml file, not just the lines Aliza proposes to add...

@jeffibm
Copy link
Member

jeffibm commented Jan 24, 2023

This PR looks good.
Just waiting for @agrare to have one final look since the PR - ManageIQ/manageiq-providers-ibm_cloud#435 is not yet merged.

@miyamotoh
Copy link
Contributor

Thanks, @jeffibm. I thought this was the pre-cursor for that ibm_cloud's 435 to work. FWIW, I did test this change alone and UI didn't break. Let's wait for @agrare. 🙂

@agrare agrare merged commit d12a081 into ManageIQ:master Feb 1, 2023
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

6 participants