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

(PUP-7042) Mark strings in graph #5580

Merged
merged 2 commits into from Apr 13, 2017

Conversation

Magisus
Copy link
Contributor

@Magisus Magisus commented Jan 31, 2017

This commit marks user-facing error and info strings in
lib/puppet/graph/* for translation.

@puppetcla
Copy link

CLA signed by all contributors.

@Magisus Magisus changed the base branch from 5.0 to master February 2, 2017 18:03
@Magisus Magisus changed the base branch from master to 4.10.x March 1, 2017 22:34
@Magisus
Copy link
Contributor Author

Magisus commented Mar 2, 2017

Closing and reopening to fix Travis.

@Magisus Magisus closed this Mar 2, 2017
@Magisus Magisus reopened this Mar 2, 2017
This commit marks user-facing error and info strings in
`lib/puppet/graph/*` for translation.
@Magisus Magisus changed the base branch from 4.10.x to master March 22, 2017 19:02
@@ -225,18 +225,18 @@ def report_cycles_in_graph
return if n == 0
s = n == 1 ? '' : 's'

message = "Found #{n} dependency cycle#{s}:\n"
message = _("Found %{num} dependency cycle%{s}:\n") % { num: n, s: s }
Copy link
Contributor

Choose a reason for hiding this comment

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

There might be some pluralization going on here (for dependency cycle)?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 - probably could use the pluralization form here

@@ -146,7 +146,7 @@ def tarjan(root, s)
frame[:step] = :children

else
fail "#{frame[:step]} is an unknown step"
fail _("%{step} is an unknown step") % { step: frame[:step] }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we should externalize this. It's such an obscure error in the graph logic that someone who comes across it in japanese will probably have better luck searching for resolution in english than with a translation that probably won't reflect the nature of the message.

@@ -193,7 +193,7 @@ def find_cycles_in_graph
# through the graph first, which are more likely to be interesting to the
# user. I think; it would be interesting to verify that. --daniel 2011-01-23
def paths_in_cycle(cycle, max_paths = 1)
raise ArgumentError, "negative or zero max_paths" if max_paths < 1
raise ArgumentError, _("negative or zero max_paths") if max_paths < 1
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest translator comment noting that negative or zero refers to integers

message += "Try the '--graph' option and opening the "
message += "resulting '.dot' file in OmniGraffle or GraphViz"
message += _("Try the '--graph' option and opening the ")
message += _("resulting '.dot' file in OmniGraffle or GraphViz")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if OmniGraffle or GraphViz should be translated - probably worth a translator note

Copy link
Contributor

@MosesMendoza MosesMendoza left a comment

Choose a reason for hiding this comment

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

per comments

Copy link
Contributor

@mcdonaldseanp mcdonaldseanp left a comment

Choose a reason for hiding this comment

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

Minor comment changes

else
message += "Try the '--graph' option and opening the "
message += "resulting '.dot' file in OmniGraffle or GraphViz"
message += _("Try the '--graph' option and opening the ")
Copy link
Contributor

Choose a reason for hiding this comment

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

--graph is a keyword, and we should leave a comment to leave it out of translation

@Magisus
Copy link
Contributor Author

Magisus commented Apr 10, 2017

Updated.

@MosesMendoza
Copy link
Contributor

@mcdonaldseanp are you good with the updated changeset?

@MosesMendoza
Copy link
Contributor

Looking at the comment @mcdonaldseanp left, it has been addressed. I think this is good to merge as he is unavailable for a bit.

@MosesMendoza MosesMendoza merged commit 1f06847 into puppetlabs:master Apr 13, 2017
@Magisus Magisus deleted the PUP-7042/graph branch November 21, 2017 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants