-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
FC-0001: Account pages -> micro-frontend #30336
Conversation
Thanks for the pull request, @UvgenGen! 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. |
@dianakhuang This seems to be with arch-bom, do you mind reviewing and merging? |
94abf32
to
d306a99
Compare
bb02e86
to
6549a21
Compare
d2aa853
to
2a54f42
Compare
@UvgenGen i have reviewed and tested your changes and resolved merge conflicts locally. I cannot push my changes in your forked repo. Please either give me write access to your repo or i'll provide the patch file for changes. |
@abdullahwaheed Could you provide the patch file please? |
@UvgenGen the patch couldn't created for all the commits as i have merged it with master and resolved conflicts only. Could you please rebase it with master and i'll help you resolve the conflicts |
2a54f42
to
a54eff5
Compare
@abdullahwaheed yes, sure. I rebased this PR and resolved conflicts. Files affected: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested with respective changes. LGTM
@UvgenGen could you please rebase it again |
a54eff5
to
6658377
Compare
@abdullahwaheed PR rebased) |
@UvgenGen should the name of this PR be updated? We're after olive, can this now be merged? |
@e0d I removed |
@dianakhuang is this good to merge now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me!
I will try to get it on FED-BOM's agenda to review and help get this merged. Very excited for this to land! |
@UvgenGen need rebasing again due to frequent changes in platform :) |
6658377
to
ed4e690
Compare
@abdullahwaheed PR rebased) |
Hi @UvgenGen @arbrandes @dianakhuang - is this good to merge once the branch conflicts are resolved? |
ed4e690
to
027e429
Compare
@mphilbrick211 PR rebased, conflicts resolved) |
@arbrandes @dianakhuang Would one of you be able to merge this? |
@UvgenGen 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
This reverts commit 0f02c7b.
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
EdX Release Notice: This PR has been deployed to the production environment. |
This reverts commit 0f02c7b.
feat: Account and profile MFE legacy removal - redeployment * Revert "Revert "FC-0001: Account pages -> micro-frontend (#30336)" (#31888)" This reverts commit 90c4ca6. * refactor: removed filters test from user_api accounts --------- Co-authored-by: Bilal Qamar <59555732+BilalQamar95@users.noreply.github.com>
Merge after the Olive release.
Description
replaced edx-platform's older implementation of the Django-server-side rendering of the following pages with new micro-frontend-based implementations:
** Account Settings
PR to frontend-platform: openedx/frontend-platform#326
PR to frontend-component-header: openedx/frontend-component-header#210
Supporting information
openedx/public-engineering#71
Original Jira Issue: https://openedx.atlassian.net/browse/DEPR-17