The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33224.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

ACK 2885bd0

The PR adjusts the -datacarrier/-datacarriersize deprecation warning to be less absolute and better match the release notes.
I think the new wording still communicates deprecation, but without overstating certainty about removal.
Code change is minimal and the functional test was updated accordingly.

Code review ACK 2885bd0. I don't think it is good for the release notes and the runtime warning message to say two different things. I'd also be happy if release notes were updated to match the runtime warning, instead of vice versa. Whatever is more accurate is better.

ACK 2885bd0

Unless there's an explicit schedule for the removal (eg -paytxfee is deprecated and will be fully removed in v31.0), this phrasing seems more accurate. Probably the testnet3 deprecation should also either be scheduled or changed to "is expected to be removed" as well.

cc @hebasto; given this would change translations after translation string freeze.

cc @hebasto; given this would change translations after translation string freeze.

The translation workflow on Transifex includes marking translated strings as "Reviewed", which locks them from further changes. Not every translation team uses this feature, but those who do rely on it. Unfortunately, the "opensource" plan used by the Bitcoin organization on Transifex has very limited functionality, and it is not guaranteed that updating the translation source file (as in #33193) won’t reset the "Reviewed" status for other strings.

In short, there must not be any translation source updates after the final translation string freeze.

@w0xlt

Will the src/qt/bitcoinstrings.cpp file be changed in the GUI repository?

It makes no difference.

@hebasto, how can we help with finding a solution?
I personally would be okay with only fixing the English version, if updating the rest is indeed an unsolvable problem.

ACK 2885bd0

Makes sense to have consistent working with the release note

Unfortunately, the "opensource" plan used by the Bitcoin organization on Transifex has very limited functionality, and it is not guaranteed that updating the translation source file (as in #33193) won’t reset the "Reviewed" status for other strings.

Can we pay them some money to make that no longer a problem?

I've done some experimenting with a test project on transifex to see what the actual effects this change would cause, and a potential workaround.

In this test project, I copied over a couple fully translated languages to observe how the translated strings change when the source is updated. After rebasing this PR branch and doing cmake --build build --target translate, I uploaded the resulting modified bitcoin_en.xlf file. This resulted in 122 (~10% of strings) being marked untranslated again in both languages. We can assume that that behavior will appear across all languages, so that means ~10% of all translated strings would need to be retranslated. That's the crux of the problem. Now, Transifex does provide suggestions based on previous translations so those translations can be filled in with the click of a button. But that means someone needs to do that for every single language. I think this issue is why we institute a translation strings freeze.

However, I think there is a workaround to this issue. Ultimately the reason transifex thinks 121 additional strings need to be translated is because the .xlf files contain an id for each string which is based upon the position of a string in the bitcoin_en.ts file which is automatically generated from the source code using lupdate. But if we instead modify bitcoin_en.ts directly as well, lupdate will not change the string order so the generated bitcoin_en.xlf file will contain just the changed string (and some other autogenerated context changes that don't seem to make a difference), which can be uploaded to Transifex. The result is that only this single string needs to be translated and no other strings will be affected.

If this doc change is desired by contributors, and the 9 acks suggested that it is, then I think this workaround of modifying bitcoin_en.ts would be suitable.

Ultimately the reason transifex thinks 121 additional strings need to be translated is because the .xlf files contain an id for each string which is based upon the position of a string in the bitcoin_en.ts file which is automatically generated from the source code using lupdate

Is this context info useful? Line numbers will change whenever some previous function in the file gets a non-trivial edit... Would it be better to add -locations none to the LCONVERT invocation in translate.cmake? (Post branch-off) Or are there instances where the same english text (in the same source file?) gets multiple different translations in a single language?

EDIT: https://wiki.qt.io/Qt_Localization seems to suggest -locations none is a good idea fwiw. Could also consider switching from tranifex to weblate, seems to be what fedora uses; mumble switched with this rationale after this discussion -- we'd be under the 10k 160k string limit but over the 60 language limit so would presumably need to pay 500 EUR/year and apparently they now offer unlimited languages so that would also be fine; and we could also self-host which is what fedora does. They seem to offer context links back to the source repo, and some degree of github integration for automating pulls/pushes.

I have added a tool which could be useful for making sure edits like this aren't problematic anymore - feedback is welcome on overall direction: #33270

Is this context info useful? Line numbers will change whenever some previous function in the file gets a non-trivial edit... Would it be better to add -locations none to the LCONVERT invocation in translate.cmake?

I don't think line numbers is the problem. The actual source location of the string can change and the ids in the .xlf won't change. It's the sort order in the .ts file that lupdate generates that seems to cause the id change. It's unclear to me why the sort order would change.

I have checked the problem against the actual Core translations on Transifex and I could reproduce the massive invalidations and #33270 (comment) does fix it successfully so that translators only need to check the modified entries:

I've done some experimenting with a test project on transifex to see what the actual effects this change would cause, and a potential workaround.

In this test project, I copied over a couple fully translated languages to observe how the translated strings change when the source is updated. After rebasing this PR branch and doing cmake --build build --target translate, I uploaded the resulting modified bitcoin_en.xlf file. This resulted in 122 (~10% of strings) being marked untranslated again in both languages. We can assume that that behavior will appear across all languages, so that means ~10% of all translated strings would need to be retranslated. That's the crux of the problem. Now, Transifex does provide suggestions based on previous translations so those translations can be filled in with the click of a button. But that means someone needs to do that for every single language. I think this issue is why we institute a translation strings freeze.

Was "Translation Memory Fillup" enabled then?

If this doc change is desired by contributors, and the 9 acks suggested that it is, then...

... it can be accepted as is, but left untranslated until the version v31.0.

This is acceptable because:

1. We never promised, nor have we ever delivered, 100% translations for every language.
2. It's still easy for the user to translate the warning message themself.

I still hesitant about introducing last-minute changes to the translation pipeline / framework. We can revisit all related suggestions, such as dropping locations and ids, right after branching off.

Was "Translation Memory Fillup" enabled then?

Only if it is by default. I don't know where that setting is.

ACK 2885bd0