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

Fixes #14256 - Update the strings to be more compliant with the proje… #156

Merged
merged 1 commit into from Apr 13, 2016

Conversation

bkearney
Copy link
Contributor

…ct standards

@@ -10,7 +10,7 @@ def update_scap_client_params(proxy_id)
new_proxy = SmartProxy.find proxy_id
model_match = self.class.name.underscore.match(/\Ahostgroup\z/) ? "hostgroup" : "fqdn"
puppetclass = Puppetclass.find_by_name("foreman_scap_client")
fail _("Puppetclass 'foreman_scap_client' not found, make sure it is imported form Puppetmaster") if puppetclass.nil?
fail _("Puppet class 'foreman_scap_client' not found, make sure it is imported form Puppetmaster") if puppetclass.nil?
Copy link
Contributor

Choose a reason for hiding this comment

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

"Puppet master" too

@domcleal
Copy link
Contributor

#: ../app/helpers/policy_dashboard_helper.rb:26
msgid "Incompliant hosts"

Should that be non-compliant?

#: ../app/models/foreman_openscap/compliance_status.rb:22
#: ../app/views/foreman_openscap/policy_mailer/_dashboard.erb:17
msgid "Incompliant"

ditto?

#: ../app/models/foreman_openscap/scap_content.rb:9
msgid "No Proxy with OpenSCAP features"
#: ../app/models/foreman_openscap/scap_content.rb:14

Bit of inconsistent capitalisation.

#: ../app/views/arf_reports/delete_multiple.html.erb:28
msgid "these Complinace reports"

Spelling.

#: ../app/views/arf_reports/show.html.erb:4
msgid "Reported at %s "

Trailing whitespace.

#: ../app/views/compliance_hosts/show.html.erb:3
msgid "%s compliance reports by policy"

Missing plural forms, use n_().

#: ../app/views/dashboard/_compliance_host_reports_widget.html.erb:12
msgid "Othered|O"
msgstr ""

Should that say "Others" or something? These should have a translator comment like Foreman core, e.g. # TRANSLATORS: initial character of Other.

#: ../app/views/policies/welcome.html.erb:11
msgid ""
"In Foreman, a compliance policy checklist is defined via %s, once SCAP content"
" is present, you can create a policy,\n"
"            assign select hostgroups and schedule to run."
msgstr ""

"hostgroups" should be "host groups", and like the other in this file (and app/views/scap_contents/welcome.html.erb), it might be best to remove the whitespace from the source string. I'm unsure what %s here is too.

@bkearney
Copy link
Contributor Author

@domcleal Made most of the changes. Some I did not:

  • Incompliant - This seems to be an SCAP word, and it is valid per Websters.
  • I could not get the comments from the html to make it into the translator.

The rest are in, and I have force pushed the changes.

@shlomizadok
Copy link
Member

@bkearney - thanks for this.
The tests are failing because of texts changes. OpenScap should be OpenSCAP in the tests (I can change this for you if you want).

@domcleal do you ack the changes?

@domcleal
Copy link
Contributor

msgid "No Proxy with OpenSCAP features"
Bit of inconsistent capitalisation.

Looks unfixed, lower case "proxy".

msgid "Othered|O"
Should that say "Others" or something?

Unfixed? Or is that more app-specific terminology?

I could not get the comments from the html to make it into the translator.

Foreman puts them into a helper, which extracts properly: https://github.com/theforeman/foreman/blob/1.11.0-RC2/app/helpers/dashboard_helper.rb#L95

@shlomizadok
Copy link
Member

"Othered" is a terminology used by OpenSCAP. We can change it though to a proper English word.

@domcleal
Copy link
Contributor

It's just if you don't change it, adding a comment to explain what it means & how it should be translated would assist translators.

@bkearney
Copy link
Contributor Author

I left Othered there for that reason. I dont know OpenSCAP well enough to to suggest changing it.

@bkearney
Copy link
Contributor Author

@domcleal that strings in in html, so I tried to add the color but it seems to have not taken it.

@domcleal
Copy link
Contributor

If it's in a view it can be put straight into a helper, then should extract fine.

@shlomizadok
Copy link
Member

@bkearney - Could you kindly change also the texts in test/unit/scap_content_test.rb (line 18) so it will fit the new texts? (and make tests pass...)
Thanks

@shlomizadok
Copy link
Member

@bkearney - would love to merge this once comments (and test) are fixed

@bkearney
Copy link
Contributor Author

bkearney commented Apr 7, 2016

@shlomizadok in.. sorry for the delay.

@bkearney
Copy link
Contributor Author

@shlomizadok bump

@shlomizadok
Copy link
Member

Thank you so much @bkearney !
Thank you @domcleal for reviewing and commenting.

@shlomizadok shlomizadok merged commit cd59dd8 into theforeman:master Apr 13, 2016
@bkearney bkearney deleted the bkearney/14256 branch August 30, 2016 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants