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

CI should detect duplicated context.json entries #7901

Closed
mlewand opened this issue Aug 21, 2020 · 6 comments
Closed

CI should detect duplicated context.json entries #7901

mlewand opened this issue Aug 21, 2020 · 6 comments
Labels
Epic squad:platform Issue to be handled by the Platform team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@mlewand
Copy link
Contributor

mlewand commented Aug 21, 2020

Provide a description of the task

Duplicated contexts should be detected by the CI, and should not be a thing that catches you off guard when preparing editor builds.

Followup of #7900

@mlewand mlewand added type:task This issue reports a chore (non-production change) and other types of "todos". squad:platform Issue to be handled by the Platform team. labels Aug 21, 2020
@mlewand mlewand added this to the nice-to-have milestone Aug 31, 2020
@mlewand
Copy link
Contributor Author

mlewand commented Oct 19, 2020

It would make to run it on our uber integration CI (because CF and external projects share the ids with CKE5).

@mlewand mlewand added the intro Good first ticket. label Oct 19, 2020
@pomek
Copy link
Member

pomek commented Oct 26, 2020

TODO: What to do if an error occurs?

@Reinmar
Copy link
Member

Reinmar commented Oct 26, 2020

TODO:

  • Document what to do when a duplicate is detected
    • If the other usage is in a package like ckeditor5-core that we know that is available (because it's a depdendency), then we can simply remove the new usage (e.g. in html-embed).
    • If the two conflicting usages are in packages like html-embed and media-embed, we cannot assume that the other on is installed, so we need to move this string to ckeditor5-core. This is tricky, because we MUST ensure that Transifex will port translations form the old resource (e.g. ckeditor5-media-embed/"Something") to the new (ckeditor5-core). We think that Transifex is configured to do that, but we cannot be sure without trying. It's e.g. CRITICAL to first download all translations yarn translations:download before doing anything so if we lose translations on Transifex we can recreate them from localhost.
    • Another interesting case is when we have two identical strings but with a different meaning. We have that case predicted in the API (the translation ids) but we never used that nor have it documented anywhere where it'd be visible. We should at least mention that such a case is possible.
  • Once we have the above documented, we should link to that page/section from the CI error.

@pomek pomek modified the milestones: nice-to-have, iteration 38 Oct 26, 2020
@pomek
Copy link
Member

pomek commented Nov 5, 2020

Yesterday I check the translations:collect script that we're using for checking translations string.

Unfortunately, it searches for packages in the packages/ directory only. We could add an option for searching in the external/ directory too. However, it will impact our downloading/uploading translation process and it seems to be dangerous.

Instead, I'd create a new script that will be searching for packages in directories: packages/, external/*/packages/. After collecting and validating stuff, the script should remove their results in order to avoid uploading anything by mistake.

The new script could be called translations:validate and should be executed on uber CI.

@pomek
Copy link
Member

pomek commented Dec 1, 2020

PR that allows checking translations in packages inside the external/ directories: ckeditor/ckeditor5-dev#678.

Some warning occurred and I reported it in proper repositories.

@pomek
Copy link
Member

pomek commented Dec 1, 2020

We need an article in our KB that will help with resolving issues if a conflict in translations occurs.

The URL to the article could be attached to output on our CI.

@pomek pomek added the Epic label Dec 1, 2020
ma2ciek added a commit to ckeditor/ckeditor5-dev that referenced this issue Dec 1, 2020
Feature (env): Added the "--include-external-directory" to the translations:collect task that allows checking packages located in the "external/" directory. See ckeditor/ckeditor5#7901.
@pomek pomek modified the milestones: iteration 39, iteration 40 Jan 11, 2021
@AnnaTomanek AnnaTomanek modified the milestones: iteration 40, nice-to-have Feb 17, 2021
@pomek pomek removed their assignment Mar 12, 2021
@pomek pomek closed this as completed Jul 13, 2021
@Reinmar Reinmar modified the milestones: nice-to-have, iteration 45 Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic squad:platform Issue to be handled by the Platform team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

4 participants