Skip to content

Parameterise command line option defaults, so translations are independent of them#5076

Merged
laanwj merged 2 commits intobitcoin:masterfrom
luke-jr:notranslate_defaults
Oct 21, 2014
Merged

Parameterise command line option defaults, so translations are independent of them#5076
laanwj merged 2 commits intobitcoin:masterfrom
luke-jr:notranslate_defaults

Conversation

@luke-jr
Copy link
Copy Markdown
Member

@luke-jr luke-jr commented Oct 10, 2014

No description provided.

@TheBlueMatt
Copy link
Copy Markdown
Contributor

utACK

src/init.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please parametrize only the '1' here, that makes for an easier translation message

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Does it? "0 if no -proxy or -connect" doesn't really make sense :/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See it from this angle: making people translate a message as a fragment that will be inserted as-is into another message is just pointless.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If the default changes, only that part will be invalidated in the translations, not the entire description?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would that be better? In both cases you have to re-translate one message. But in the case of the fragment it makes much less sense to the translator.

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Oct 11, 2014

ACK apart from the nits

@luke-jr luke-jr force-pushed the notranslate_defaults branch from 9b5946d to 0a08aa8 Compare October 11, 2014 08:23
@Diapolo
Copy link
Copy Markdown

Diapolo commented Oct 11, 2014

That is better, but I'm missing constants to replace those default values in the code!

src/init.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not your mistake, but I only notice this now: this should be '1 if no <mode> specified'. Let's leave default out of this. If 1 would really be the default, it would always do a zapwallettxes operation!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed

@luke-jr
Copy link
Copy Markdown
Member Author

luke-jr commented Oct 14, 2014

Making constants makes more discussion. Let's do that in a follow-up PR after this is merged.

@Diapolo
Copy link
Copy Markdown

Diapolo commented Oct 15, 2014

Why? Just because you want to hurry ;).

@luke-jr
Copy link
Copy Markdown
Member Author

luke-jr commented Oct 15, 2014

Because it's better to do things in small steps so there's less to rebase when one part of it prompts months of discussions. And this gets one improvement out there sooner if the other one pushes past a release. There's nothing inherently related about variablising stuff here.

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Oct 15, 2014

Agree with @luke-jr here. Right now this is an extremely low-impact change, and reviewing is limited to the documentation only. If you were to make constants and use those it is more work to verify that everything is unchanged.

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Oct 15, 2014

Tested ACK

@laanwj laanwj merged commit c0195b1 into bitcoin:master Oct 21, 2014
laanwj added a commit that referenced this pull request Oct 21, 2014
c0195b1 Bugfix: Remove default from -zapwallettxes description (inaccurate) (Luke Dashjr)
0a08aa8 Parameterise command line option defaults, so translations are independent of them (Luke Dashjr)
@luke-jr luke-jr deleted the notranslate_defaults branch July 2, 2015 23:42
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants