Internationalization process#206
Conversation
| * Created by alex on 08/11/2017. | ||
| */ | ||
| class ActiveGamesRepository( | ||
| private val context: Context, |
There was a problem hiding this comment.
Please don't inject context into the repositories, the repos should be Android agnostic. There is no point in translating log messages, in fact it can be detrimental as I may get Firbase logs in several languages that I may not understand.
| if (gameDao.getGameNullable(game.id) == null) { | ||
| FirebaseCrashlytics.getInstance() | ||
| .log("ActiveGameRepository: New game found from active_game notification ${game.id}") | ||
| .log( |
There was a problem hiding this comment.
please don't translate logs, only strings that are visible to the user.
| val localGames = games.map(Game.Companion::fromOGSGame) | ||
| gameDao.insertAllGames(localGames) | ||
| FirebaseCrashlytics.getInstance().log("overview returned ${localGames.size} games") | ||
| FirebaseCrashlytics.getInstance().log( |
There was a problem hiding this comment.
please don't translate logs, only strings that are visible to the user.
| private suspend fun updateGamesThatFinishedSinceLastUpdate(gameIds: List<Long>) { | ||
| FirebaseCrashlytics.getInstance() | ||
| .log("Found ${gameIds.size} games that are neither active nor marked as finished") | ||
| .log( |
There was a problem hiding this comment.
please don't translate logs, only strings that are visible to the user.
| override val highlighted: Boolean = false | ||
| ) : BottomBarButton { | ||
| ) : BottomBarButton, KoinComponent { | ||
| private val context: Context by inject() |
There was a problem hiding this comment.
please don't inject the Context like this, you don't need it, as Compose has stringResource() for this exact purpose
| }, | ||
| text = { Text("That move would repeat the board position. That's called a KO, and it is not allowed. Try to make another move first, preferably a threat that the opponent can't ignore.") }, | ||
| title = { Text("Illegal KO move", style = MaterialTheme.typography.titleLarge) }, | ||
| text = { Text(stringResource(R.string.that_move_would_repeat_the_board_position_that_s_called_a_ko_and_it_is_not_allowed_try_to_make_another_move_first_preferably_a_threat_that_the_opponent_can_t_ignore)) }, |
There was a problem hiding this comment.
please use shorter kets for translation such as R.string.ko_description
| import kotlin.coroutines.cancellation.CancellationException | ||
|
|
||
| class MyGamesViewModel( | ||
| private val context: Context, |
There was a problem hiding this comment.
no Context inside ViewModel please
|
Hi @acristescu ! I reviewed the comments and made the necessary corrections. I'm not familiar with Java or Kotlin (I'm from the .NET world), so please excuse any conceptual errors I'm making in the project. I'll be more attentive to these issues so as not to overload you with work. Thanks for the feedback. |
2 participants
Hi! I'm working on the internationalization process.
My goal is to only modify the bare minimum necessary to extract the hardcoded text and add it to the strings.xml file, although in some cases I had to tweak the code a bit (for example, I had to inject the Context into ActiveGamesRepository to use the functionality to read the resource files).
I'm linking the commits to ticket 155.
There's still a lot of work to do, and I'll be making progress as quickly as work and family obligations allow.
NOTE: There may be repeated texts that I've saved in strings.xml with different keys because I understood that the context changed; there may also be cases where the context is the same. Feel free to comment and correct me in either case.
PS: Excellent project!