Skip to content

User-facing content and codebase doc fixups from transifex translator feedback #24367

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 22, 2022
Merged

User-facing content and codebase doc fixups from transifex translator feedback #24367

merged 2 commits into from
Feb 22, 2022

Conversation

jonatack
Copy link
Member

Closes #24366.

@jonatack
Copy link
Member Author

(When to use cannot vs can not: https://writingexplained.org/cannot-or-can-not-difference. The cases when "can not" would be used are somewhat rare and not encountered here.)

@jonatack jonatack mentioned this pull request Feb 17, 2022
Closed
3 tasks
@jonatack jonatack changed the title User-facing content fixups from transifex translator feedback User-facing content and codebase doc fixups from transifex translator feedback Feb 17, 2022
@ghost
Copy link

ghost commented Feb 17, 2022 •
edited by ghost
Loading

(When to use cannot vs can not: https://writingexplained.org/cannot-or-can-not-difference. The cases when "can not" would be used are somewhat rare and not encountered here.)

Based on this link: https://english.stackexchange.com/questions/4510/why-is-cannot-spelled-as-one-word both look okay to me.

Overall I have no issues with the changes made in this PR.

ACK 4982b32

hebasto
hebasto reviewed Feb 17, 2022
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Concept ACK.

hebasto
hebasto approved these changes Feb 17, 2022
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 4982b32

@jonatack
Copy link
Member Author

(TBH I'd noticed most of this diff before but thought they were too pedantic to propose changing. To my mind this is more to avoid spending further time on it.)

@hebasto
Copy link
Member

hebasto commented Feb 17, 2022

(TBH I'd noticed most of this diff before but thought they were too pedantic to propose changing. To my mind this is more to avoid spending further time on it.)

Translators forms a good focus-group for Bitcoin Core UI testing :)

@hebasto hebasto mentioned this pull request Feb 18, 2022
Merged
w0xlt
w0xlt approved these changes Feb 18, 2022
View reviewed changes
Copy link
Contributor

@w0xlt w0xlt left a comment

Choose a reason for hiding this comment

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

ACK 4982b32

luke-jr
luke-jr reviewed Feb 19, 2022
View reviewed changes
Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

I think the current strings are better, and don't see rationale for changing them (exception ACK'd)

src/wallet/spend.cpp
@@ -778,7 +778,7 @@ static bool CreateTransactionInternal(
}
else if ((unsigned int)nChangePosInOut > txNew.vout.size())
{
error = _("Change index out of range");
Copy link
Member

Choose a reason for hiding this comment

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

ACK

@jonatack
Copy link
Member Author

I think the current strings are better, and don't see rationale for changing them (exception ACK'd)

That leaves

  • "free space available", which I agree may not bother native speakers (it doesn't bother me, at least) but I can see why it may appear redundant to non-natives / English-as-a-second language speakers (as seen in the referenced issue in the PR description), so we may as well avoid the issue coming up
  • "can not", for which there are explanations and links above

@jonatack
Copy link
Member Author

Summary: this has 3 ACKs from @prayank23, @hebasto and @w0xlt

@laanwj
Copy link
Member

laanwj commented Feb 21, 2022 •
edited
Loading

Code review ACK 4982b32
Code review re-ACK 4874269
But would prefer the change in build-unix.md to be reverted, it's better to quote error messages verbatim even if they contain typos. (addressed)

@jonatack jonatack force-pushed the fixes-based-on-transifex-translator-feedback branch from 4982b32 to 4874269 Compare February 21, 2022 18:08
@jonatack
Copy link
Member Author

Updated per review feedback to leave the error example as-is in doc/build-unix.md. Thanks!

hebasto
hebasto approved these changes Feb 21, 2022
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK 4874269, only suggested change since my previous review.

@laanwj laanwj merged commit 8add59d into bitcoin:master Feb 22, 2022
maflcko
maflcko reviewed Feb 22, 2022
View reviewed changes
src/wallet/spend.cpp
@@ -778,7 +778,7 @@ static bool CreateTransactionInternal(
}
else if ((unsigned int)nChangePosInOut > txNew.vout.size())
{
error = _("Change index out of range");
error = _("Transaction change output index out of range");
Copy link
Member

Choose a reason for hiding this comment

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

Is there any value in translating this? I mean it is not possible to set the change position via the GUI, no?

@jonatack jonatack deleted the fixes-based-on-transifex-translator-feedback branch February 22, 2022 15:25
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 22, 2022
@laanwj
Merge bitcoin#24367: User-facing content and codebase doc fixups from…
88bd0e3 
… transi…
@bitcoin bitcoin locked and limited conversation to collaborators Feb 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@laanwj laanwj laanwj left review comments

@hebasto hebasto hebasto approved these changes

+3 more reviewers

@luke-jr luke-jr luke-jr left review comments

@maflcko maflcko maflcko left review comments

@w0xlt w0xlt w0xlt approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
GUI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Transifex translators feedback
7 participants
@jonatack @hebasto @laanwj @luke-jr @maflcko @w0xlt @DrahtBot