Skip to content

Conversation

@benediktjohannes
Copy link

@benediktjohannes benediktjohannes commented Jan 15, 2026

Contributor checklist

  • Device A, Android X.Y.Z
  • Device B, Android Z.Y
  • Virtual device W, Android Y.Y.Z
  • [x ] My contribution is fully baked and ready to be merged as is
  • [ x] I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Fixes an issue with a wrong string (which is not a string itself, but a comment in strings.xml (so not a translation and it has to be done via GitHub because its something inside a string))

Fixes #14533

Fixes an issue with a wrong string (which is not a string itself, but a comment in strings.xml (so not a translation and it has to be done via GitHub because its something inside a string))
Copy link
Contributor

@mtang-signal mtang-signal left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the attention to details. Two notes:

  • Could you combine all the pull requests into this one so it can be reviewed as one commit? If you want, you can use separate commits.
  • If you are change the internal value of the string (eg #14539), you need to also change where it gets called in the codebase. Otherwise, it will cause a crash because it can no longer find the resource (since it was renamed).

@benediktjohannes
Copy link
Author

Hi, thank you for the review and the friendly answer! The fact that there could be crashes is a very important thing to mention (I also thought about that already and mentioned it here indirectly: Fixes typing: Important this is not a translation change, this is an internal change of the value field for the translations - another important thing to mention: This (probably) has to be changed in all language translation files as well and also in the general value context if it's wrong there as well and copied). And I already did some of those changes as well in the other Signal repo for Apple devices, but it's just that many so that it takes some time for me (unforetunately). But there are many cases as well where occour crashes already at the moment (probably) because there are wrong messages only in one of the files and not in the other and then there is probably something missing as well, but you're definitely right that attention is important there, thanks for mentioning this again!

In this specific case (just the change of description) this should not be relevant because it's only a kind of commentary, but generally (as you correctly mentioned in #14539) you're totally right!

And one problem there is still, unforetunately: I do only use GitHub online and not Git itself and I think that there is no ability to force push branches (at least I don't know about it). And I'm quite new to GitHub (so I don't really know how to combine this in one, I'm really sorry). But of course, I can combine those things in future pull requests in one (as I already did in the iOS Signal project).

@benediktjohannes
Copy link
Author

For example the typos in iOS only existed in two pathes leading to the fact that in the other cases there could be crashed because of the missing wrong spelled messages in the current repository (so I think (if it really crashed and it's not only an error), then this should be fixed as soon as possible). Thank you!

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.

Wrong comment " <!-- Content descrption for name edit button on contact share edit activity -->"

2 participants