fheroes2 icon indicating copy to clipboard operation
fheroes2 copied to clipboard

Italian Translation improvement

Open g1augusto opened this issue 1 year ago • 7 comments

Hi,

I reworked the Italian translation as you suggested, with POedit.

At the moment is at 63% but I think that the actual percentage may be higher

g1augusto avatar Aug 26 '24 19:08 g1augusto

Hi @zenseii ,

I just applied the changes you advised, let me know if it's good now

g1augusto avatar Aug 27 '24 09:08 g1augusto

@g1augusto. Thanks for addressing the comments. The PR is still waaay too big for Github to handle properly, and this will only make the review process take much much longer.

We need to portion this PR into smaller PRs, around total 100 changed lines, which means 50 added and 50 removed lines. Here's a simple step-by-step guide:

  1. Keep this branch and copy the it.po file to your desktop
  2. Create a new branch
  3. Copy paste around 50 changes from the it.po file on your desktop to the new branch's it.po file.
  4. Make a new PR

zenseii avatar Aug 27 '24 13:08 zenseii

OK, so I will make a new branch and pull request for each 50 lines changes? Will that be something like 10+ of them?

g1augusto avatar Aug 27 '24 13:08 g1augusto

OK, so I will make a new branch and pull request for each 50 lines changes? Will that be something like 10+ of them?

50 changed lines will be around 25 copied strings. I say around because it depends on the string in question.

zenseii avatar Aug 27 '24 13:08 zenseii

I see, but don't you think that this hard requirement doesn't favor translations? I understand you apply this type of syntactic control on code, makes sense but translations are verbose, forcing me to make so many branches and pull requests is a little...counterproductive, isn't it?

g1augusto avatar Aug 27 '24 13:08 g1augusto

@g1augusto

I see, but don't you think that this hard requirement doesn't favor translations?

I understand you apply this type of syntactic control on code, makes sense but translations are verbose,

We only apply this to translation PRs, not to code. However, it is always better practice to make smaller targeted changes than to stack lots of different things together. This is more tidy and easier to reference back to later if necessary.

Here is an example of a PR which was before we set this requirement for translation PRs: https://github.com/ihhub/fheroes2/pull/4810

If there hadn't been 5 people working on that PR it would've taken much longer for it to finally be approved and merged.

As you can see, the more changes in one PR the more discussions arise and the harder it becomes to navigate that PR in Github. So, the main reasons we do this is because once you reach a large amount of changes, Github will take much longer to load these changes in the Files Changed tab (It will ask you to Load diff). Furthermore, if I leave more than 10 requests for changes, Github will start to hide some of the requests and you will have to constantly go back and forth trying to find a discussion you've been tagged in that was hidden. As I said before, this is mainly a Github issue.

In the case of your current PR, I estimate that I would currently have to leave 100+ requests for changes, many mistakes of which are better if you address one case of and then don't do the next PR without me having to note every instance of in the current PR. Doing PRs in small increments means that there will be this sort of learning process, instead of piling all mistakes at once in 2000 changes.

I do a lot of language reviews on my phone, and having to load large diffs on my phone every time would be a very lengthy process and ultimately would limit the opportunities I get to make reviews, which consequently makes the review process take longer.

I hope this gave some idea as to why this system has been put in place. We have reviewed countless translation PRs and this is the most efficient workflow we have come up with so far.

Here is our translation PR restrictions found in our guide: image

zenseii avatar Aug 27 '24 14:08 zenseii

Ah yes, and one more very important point is that the longer a translation PR takes to get approved and merged, the more merge conflicts you will have to solve because we are constantly changing the code and this means strings will be moved around or changed. Our automatic po-file updater will change the PO files automatically and your PR will have to grab these changes every time this happens and solve merge conflicts and if we can avoid having to do these manually then we'll have saved time.

zenseii avatar Aug 27 '24 14:08 zenseii

I have one more question: So when I create these tens of branches...do I have to add on each one of them only 50 lines of difference? or say that the 3rd branch I create would have 50+50+50 lines changed?

I think you mean that each time I have to add only 50 lines changed from the current master, which means I have to:

  1. take the current master file for each branch
  2. take a bit of what I have already worked and replace it on that specific branch
  3. commit the branch
  4. Create a pull request
  5. Goto number 1 and repeat

Is that correct?

g1augusto avatar Aug 27 '24 19:08 g1augusto

I have one more question: So when I create these tens of branches...do I have to add on each one of them only 50 lines of difference? or say that the 3rd branch I create would have 50+50+50 lines changed?

I think you mean that each time I have to add only 50 lines changed from the current master, which means I have to:

  1. take the current master file for each branch
  2. take a bit of what I have already worked and replace it on that specific branch
  3. commit the branch
  4. Create a pull request
  5. Goto number 1 and repeat

Is that correct?

Yes, the steps you provided are correct. As long as you've set up your local fork to be contributing to the ihhub master, then whenever you create a new branch locally (provided you have internet), it will already contain your last merged PR from Master. Just wait to make the new branch until the automatic "update translations" PR has been merged. This will be done very shortly after your PR is merged.

Btw for your first PR I suggest 50 lines. For all the following PRs you can make them 400 lines as shown in the guide.

I hope that answers your question. Don't hesitate to ask if you have any other doubts.

zenseii avatar Aug 27 '24 20:08 zenseii

Understood, then let's do this way, I will cancel (again) this pull request, makes no sense at this point, I will work on my own on this translation and then I will create another branch time by time (over one year... :) ) and eventually make a pull request with 50 or later 400 lines...

OK?

g1augusto avatar Aug 27 '24 20:08 g1augusto

Understood, then let's do this way, I will cancel (again) this pull request, makes no sense at this point, I will work on my own on this translation and then I will create another branch time by time (over one year... :) ) and eventually make a pull request with 50 or later 400 lines...

OK?

Yes, feel free to choose whichever approach suits you best.

zenseii avatar Aug 29 '24 13:08 zenseii

As per the PR author's last post, this PR can be closed.

zenseii avatar Sep 22 '24 09:09 zenseii