Skip to content

Conversation

@RandyMcMillan
Copy link
Contributor

@RandyMcMillan RandyMcMillan commented Apr 24, 2021

Some guidance on this would be appreciated until I get comfortable with this.

@jonatack
Copy link
Member

The PR description in #287 looks like helpful guidance for this.

(PS - don't put @-prefixed usernames in commits and PR descriptions; the latter because usernames in the description are copied into the merge commit by the merge script. This can cause endless annoying notifications for those concerned. An update to the merge script now warns maintainers if a merge message contains a @username, but it's better not to add them to begin with. For more, see https://jonatack.github.io/articles/how-to-contribute-pull-requests-to-bitcoin-core#pr-descriptions.)

@RandyMcMillan RandyMcMillan marked this pull request as draft April 24, 2021 23:33
@hebasto
Copy link
Member

hebasto commented Apr 25, 2021

@RandyMcMillan

Thanks for working on this!

The PR description in #287 looks like helpful guidance for this.

Indeed, for cases in this PR the translator comments should be used.

To ensure, that they are working properly, you could use make -C src translate.

Also for such changes I'd prefer a pr with a wide scope :)

@hebasto
Copy link
Member

hebasto commented Apr 25, 2021

@RandyMcMillan

If your goal is resolving #288, you could look into #220 where some disambiguation strings were introduced, and they should be replaced with translator comments.

return strList.join(" & ");
else
return QObject::tr("None");
return QObject::tr("None", "absence of value");
Copy link
Member

Choose a reason for hiding this comment

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

Our goal is to replace useless (in most cases) disambiguation strings (the second argument in tr call) with translator comments:

Suggested change
return QObject::tr("None", "absence of value");
//: Absence of value.
return QObject::tr("None");

@RandyMcMillan
Copy link
Contributor Author

NOTE: Currently I am just trying to get an idea of how much work needs to be done.

@hebasto
Copy link
Member

hebasto commented May 27, 2021

In its current state this PR does not follow our the most recent translation policy, as we are not using the second argument of the QObject:tr function.

#311 is a good example of the mentioned approach.

Closing for now.

@hebasto hebasto closed this May 27, 2021
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants