search

artfulrobot / uk.artfulrobot.civicrm.gocardless

A CiviCRM extension providing GoCardless integration to handle UK Direct Debits.
GNU Affero General Public License v3.0
5 stars 18 forks source link

use 'domain' argument in ts() #57

Closed twomice closed 5 years ago

twomice commented 5 years ago

Hi @artfulrobot . This is the only issue I've found in the extension review so far. It's not a requirement, but I recommend addressing it, as it can help translators to give non-English speakers a better experience.

Extension developers are asked to pass the second 'domain' argument in ts() function calls, to help translators distinguish between core terms and extension-specific terms. Reference: https://docs.civicrm.org/dev/en/latest/translation/extensions/#for-developers-correct-usage-of-the-ets-function Extensions built with newer versions of civix can use the E::ts() method to save some keystrokes and avoid typing that second parameter every time.

artfulrobot commented 5 years ago

awesome, I will get on to that. As GC is primarily relevant in UK I have not prioritised translation, which probably calls me out on anglocentric privilege!

great news about the extension's inclusion, thanks for your work.

Rich

artfulrobot commented 5 years ago

@twomice How's this looking?

I've not wrapped internal exception message strings in E::ts(), nor log messages, as they are only meant for devs (who have to struggle with English in function, variable names anyway etc) - is that OK or not?

twomice commented 5 years ago

That looks great! I added a comment where it looks like you missed one, but yes this looks like a big improvement. For finer points on translation, @ mlutfy is usually willing to help (and definitely knows what he's talking about). Thanks!

artfulrobot commented 5 years ago

Thanks, I'll close this now.