This somewhat of a continuation of this thread:
http://drupal.org/node/1932682
In that thread, I had to manually add the mapping for Chinese Tradition to work correctly. However, I wanted to upgrade to the recent DEV versions to see if these had been fixed.
As of the dev, I could not see that the mapping code had been added to tmgmt_mygengo.plugin.inc, and I was getting the same errors as the recommended release. I then went in to manually add the mapping back to line 62, but this did work. I now get the following error:
"Job has been rejected with the following error: #1551: language service not supported"
screenshot attached.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | added-traditional-chinese-1551-7.patch | 406 bytes | issam.zeibak |
| #3 | Fixed_chinese_target_language-1932684-3.patch | 854 bytes | issam.zeibak |
| #1 | Fixed_chinese_target_language-1932684-1.patch | 606 bytes | issam.zeibak |
| Screen shot 2013-03-04 at 12.05.50 PM.png | 179.61 KB | Gastonia |
Comments
Comment #1
issam.zeibak commentedThis is my first patch to commit to drupal. This should work, there target language was simply not mapped.
Comment #2
berdirCan you check if the patch in #1901890: Make use of the new remote language mapping feature from tmgmt core fixes this problem as well?
Comment #3
issam.zeibak commentedI'll check it later, but the patch does not cover both simplified and traditional chinese. I attached a new patch to cover this.
Comment #4
berdirIt's possible that the other patch is missing mappings but this should then be added to the patch there. That patch is the way forward as it implements the improved mapping functionality provided by TMGMT instead of implementing a custom solution. The patch also allows users to define their own mappings if they e.g. have custom languages defined on a site.
Comment #5
issam.zeibak commentedI'll try your other patch then. I agree better to come with drupal than to customize.
Comment #6
berdirHave you been able to test the other issue/patch to make sure that it works at least as well as the current code? I'd be more than happy to commit it and some improved default language mapping definitions (in the patch if you want to re-roll or as a follow-up here) but I don't have time to test it myself right now.
Comment #7
issam.zeibak commentedI have tested this and while it did require the extra traditional chinese mapping, it did work well. I attached the extra mapping in a patch that should be applied on top of the previous one. Thanks.
Comment #8
berdirI've committed the other issue, thanks for testing.
Looks like your patch was still against the current version and not that patch, I've applied and commited it manually.
Note: Please set the issue status to "Needs review" when you upload a patch.
Comment #9
issam.zeibak commentedThank you for that merge Berdir. Also, thank you for your catch on the Drupal version and your comment on the status change. I will be more careful going forward.