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 PxeServer.verify_depot_settings_queue #21013

Merged
merged 1 commit into from
May 14, 2021

Conversation

lfu
Copy link
Member

@lfu lfu commented Feb 4, 2021

Blocks ManageIQ/manageiq-api#1017

ManageIQ/manageiq-api#926

@miq-bot add_label enhancement, kasparov/no

app/models/pxe_server.rb Outdated Show resolved Hide resolved
app/models/pxe_server.rb Outdated Show resolved Hide resolved
app/models/pxe_server.rb Outdated Show resolved Hide resolved
@lfu lfu force-pushed the pxe_verify_credentials_926 branch 2 times, most recently from 2ac0f36 to b60096d Compare February 5, 2021 02:45
@@ -24,6 +24,21 @@ def verify_depot_settings(settings)
res
end

def verify_depot_settings_queue(userid, options)
task_opts = {
:action => "Verify #{display_name} Credentials",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is i18n compliant. @mzazrivec can you remind me of the right way to do this?

Copy link
Member

@Fryguy Fryguy Feb 5, 2021

Choose a reason for hiding this comment

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

I think it's

Suggested change
:action => "Verify #{display_name} Credentials",
:action => _("Verify %{display_name} Credentials") % {:display_name => display_name}

Copy link
Contributor

Choose a reason for hiding this comment

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

@Fryguy correct

Copy link
Member Author

Choose a reason for hiding this comment

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

This action would be logged into the log file and saved into task's message column. Do we really need to make it I18N compliant? @Fryguy

Copy link
Contributor

Choose a reason for hiding this comment

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

If the message ends up in a log file, then no, we're not translating that.

@Fryguy
Copy link
Member

Fryguy commented Feb 5, 2021

@bdunne Can you please also review?

@lfu lfu force-pushed the pxe_verify_credentials_926 branch from edcc516 to d892d49 Compare March 1, 2021 17:09
@miq-bot
Copy link
Member

miq-bot commented Mar 1, 2021

Checked commit lfu@d892d49 with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@agrare
Copy link
Member

agrare commented Mar 29, 2021

@bdunne can you take another look?

@bdunne bdunne merged commit 19f4e56 into ManageIQ:master May 14, 2021
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