Skip to content
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.

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

🌐 N°7932 Translations - British English #677

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

larhip
Copy link
Contributor

@larhip larhip commented Nov 7, 2024

Base information

Question Answer
Combodo ticket? N°7932
Type of change? Translations

Symptom (bug) / Objective (enhancement)

New language files for British English

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have tested all changes I made on an iTop instance
  • I have added a unit test, otherwise I have explained why I couldn't
  • Is the PR clear and detailed enough so anyone can understand digging in the code?

@larhip larhip changed the base branch from develop to support/3.2.0 November 7, 2024 15:34
@larhip larhip changed the base branch from support/3.2.0 to develop November 7, 2024 15:35
@larhip larhip changed the base branch from develop to support/3.2.0 November 7, 2024 15:35
@larhip larhip force-pushed the feature/translations_en_gb branch from 82bcf68 to 8c254ca Compare November 7, 2024 15:43
@larhip larhip changed the base branch from support/3.2.0 to develop November 7, 2024 15:44
@larhip larhip force-pushed the feature/translations_en_gb branch from 8c254ca to f8587d2 Compare November 7, 2024 15:46
@larhip larhip changed the base branch from develop to support/3.2.0 November 7, 2024 15:47
@larhip larhip force-pushed the feature/translations_en_gb branch from f8587d2 to d8a7d0c Compare November 7, 2024 15:50
@Hipska
Copy link
Contributor

Hipska commented Nov 7, 2024

@larhip You branched from support/3.2, then you should also select this as base branch on the PR.

@larhip larhip changed the base branch from support/3.2.0 to support/3.2 November 7, 2024 15:55
@larhip
Copy link
Contributor Author

larhip commented Nov 7, 2024

@Hipska thanks for the hint!
I was sitting at my screen thinking "What the hell is the problem?"
Every time I tried to change the base branch from develop to support 3.2, there were a lot more changes and commits than I expected.
But the only problem was related to the fact that I selected 3.2.0 instead of 3.2.

Thanks again for the hint :-)

@larhip larhip marked this pull request as ready for review November 7, 2024 15:57
Comment on lines 8 to 21
* This file is part of iTop.
*
* iTop is free software; you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* iTop is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with iTop. If not, see <http://www.gnu.org/licenses/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this part can be removed from each file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created the translation with the support of a little script. This script copies the original English files and checks every file line by line.

From my point of view it would be a good idea to have the same content and line numbering in en an en_gb files, wouldn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm not sure, for example the EN GB could have other @author references than the EN US versions, so line numbers will be hard to match anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's good to include only the copyright and license line. That said, it's not bad if files contain more information on licenses. I will apply the suggested modifications as they make sense.

dictionaries/en_gb.dictionary.itop.core.php Outdated Show resolved Hide resolved
@jf-cbd
Copy link
Contributor

jf-cbd commented Nov 8, 2024

Accepted in functional review, thanks for the contribution @larhip ! We'll see in technical review if some changes are needed.

Copy link
Contributor

@Hipska Hipska left a comment

Choose a reason for hiding this comment

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

Almost good regarding the copyright and license! Didn't check the actual translations yet, will do that at a later time.

Didn't realise there are so many inconsistencies in these dict files. But when starting from scratch for a new language, we should better get them all right and aligned from the start 😉

// You should have received a copy of the GNU Affero General Public License
// along with iTop. If not, see <http://www.gnu.org/licenses/>
/**
* @author Benjamin Planque <benjamin.planque@combodo.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

Did @BenGrenoble create this translation?

Copy link
Member

Choose a reason for hiding this comment

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

No. Its must be a copy past from another dict file.

Copy link
Contributor

@Hipska Hipska Nov 12, 2024

Choose a reason for hiding this comment

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

@larhip please fix 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sure it is a copy of the English dictionary file :-)
As I wrote previously I did not create those translations all by hand but used a script which copies all en files an go through every line to check whether there is the need to change it in British English.

This makes it pretty easy for me to update all translations (I.e. with a new version of iTop).

As soon as I will change all those requested change by you @Hipska it will become a lot more work to create new translations with a new version.

In this case it would be great to have a feedback from the iTop maintainer which means Combodo. @BenGrenoble maybe you as PO could give us your opinion or make a decision between more or less “automated” British English translations and perfectly clean files (which is currently not the case for EN US)?

Don’t get me wrong: for me both is absolutely fine, but in case of preferring clean files (even if this is not the case for en.dict.*) there might be a higher risk that British English will be not that good maintained in case of new translations.

hope you got my point 😊

Copy link
Contributor

Choose a reason for hiding this comment

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

I would not worry for new translations (dict items) as you are stating, since Combodo maintainers run an automated tool to copy these new items to the other languages and marks them to be translating by adding ~~ suffix, so everybody knows its a dict item that still needs to be translated.

Copy link
Member

Choose a reason for hiding this comment

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

You're right we will run an automated script on these dict.
For me it's ok if the names or the legal informations in the headers are not modified. As long as the format of the file is ok and the translation are good.
We've the history of the commit and @jf-cbd will double check while integrating the PR.

@BenGrenoble BenGrenoble changed the title 🌐 Translations - British English 🌐 N°7932 Translations - British English Nov 12, 2024
@jf-cbd
Copy link
Contributor

jf-cbd commented Nov 15, 2024

Accepted in technical review. I'll perform the necessary edits.

@jf-cbd jf-cbd self-assigned this Nov 15, 2024
@jf-cbd
Copy link
Contributor

jf-cbd commented Nov 15, 2024

Hello @larhip, I don't have the rights to push on your branch, so I made one with your modifications and some editions. Could you please merge my branch in your branch so I can accept the PR please ?

@larhip
Copy link
Contributor Author

larhip commented Nov 16, 2024

Hey @jf-cbd ,
Thanks a a lot for your efforts!

Since my fork is done in an organization (itomig-de/iTop instead of larhip/iTop) it is somehow impossible to allow you to modify this PR, we discussed that already in the past with Guillaume. It seems to be a restriction of GitHub.
Anyhow, your modifications are merged now, so you should be able to accept this PR.

Thanks again! :-)

@jf-cbd
Copy link
Contributor

jf-cbd commented Nov 18, 2024

Hey @larhip, thanks for the explanation :)
I did a last update to make some tests pass. May you please merge once more ? After this, I'll accept the PR.
Thanks !

@larhip
Copy link
Contributor Author

larhip commented Nov 18, 2024

@jf-cbd modifications merged :-)

@jf-cbd jf-cbd merged commit 596e26a into Combodo:support/3.2 Nov 18, 2024
1 check passed
@larhip larhip deleted the feature/translations_en_gb branch November 19, 2024 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants