openedx / frontend-app-discussions

A React-based micro frontend for the Open edX discussion forums.
GNU Affero General Public License v3.0
6 stars 62 forks source link

feat: use `atlas` in `make pull_translations` #502

Closed OmarIthawi closed 1 year ago

OmarIthawi commented 1 year ago

Changes

Testing

References

This pull request is part of the FC-0012 project which is sparked by the Translation Infrastructure update OEP-58.

Up-to-date project overview and details are available in the Approach Memo and Technical Discovery: Translations Infrastructure Implementation document.

Join the conversation on Open edX Slack #translations-project-fc-0012.

Check the links above for full information about the overall project.

Internalization is being rearchitected in Open edX Python, XBlock, Micro-frontend, and other projects. There are a number of immediately visible changes:

Breaking Changes

One of the primary goals of the project is to avoid breaking changes. If you noticed any suspicious code, please raise your concern. But before that, please know the strategy we're following to avoid breaking changes:

For Micro-frontends:

openedx-webhooks commented 1 year ago

Thanks for the pull request, @OmarIthawi! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (b2b33b7) 91.88% compared to head (2c186d1) 91.88%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #502 +/- ## ======================================= Coverage 91.88% 91.88% ======================================= Files 170 170 Lines 3474 3474 Branches 905 905 ======================================= Hits 3192 3192 Misses 263 263 Partials 19 19 ``` | [Impacted Files](https://app.codecov.io/gh/openedx/frontend-app-discussions/pull/502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx) | Coverage Δ | | |---|---|---| | [src/index.jsx](https://app.codecov.io/gh/openedx/frontend-app-discussions/pull/502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx#diff-c3JjL2luZGV4LmpzeA==) | `0.00% <ø> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

e0d commented 1 year ago

@OmarIthawi looks like a conflict was introduced, can you have a look?

OmarIthawi commented 1 year ago

@e0d the conflicts seems to be resolve for other reasons.

brian-smith-tcril commented 1 year ago

@jristau1984 @asadazam93 the spreadsheet says to inform you before merging this. It's ready to go, any reason I should hold off?

brian-smith-tcril commented 1 year ago

@OmarIthawi looks like this has a conflict again

OmarIthawi commented 1 year ago

@brian-smith-tcril done. cc: @jristau1984 @asadazam93

brian-smith-tcril commented 1 year ago

@OmarIthawi looks like tests are failing on this one now

OmarIthawi commented 1 year ago

Those in principle, shouldn't fail. But I'm looking into them anyway.

asadazam93 commented 1 year ago

@OmarIthawi we have another PR that upgrades the frontend platform version. I think it would make sense to merge that one first.

OmarIthawi commented 1 year ago

@asadazam93 yes please, I'd love it if that happens soon :)

brian-smith-tcril commented 1 year ago

I'm marking this as "blocked by other work" until https://github.com/openedx/frontend-app-discussions/issues/518 is resolved. I'll revisit next week and if the failing tests on master are still not resolved I'll consider merging it with failing tests.

OmarIthawi commented 1 year ago

Yes, it's ready for merge @awais-ansari. Thanks for checking.

openedx-webhooks commented 1 year ago

@OmarIthawi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.