Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

fix: enable frontend-app-learning by default #880

Closed
wants to merge 2 commits into from
Closed

fix: enable frontend-app-learning by default #880

wants to merge 2 commits into from

Conversation

ztraboo
Copy link
Contributor

@ztraboo ztraboo commented Jan 12, 2022 •
edited

Ran into this issue when trying to access the new frontend-app-learning from the maple.master devstack install. Used #865 to fix the issue.

microfrontend-issue-mozjpeg

Made frontend-app-learning as a default repo to install on devstack.

@pshiu @ztraboo
Merge pull request openedx-unsupported#865 from edx/pshiu/add-zlib-to…
8a8e421 
…-devstack

fix: bump node image for microfrontends for mozjpeg-bin
@openedx-webhooks
Copy link

Thanks for the pull request, @ztraboo! I've created OSPR-6380 to keep track of it in JIRA, where we prioritize reviews. 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:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

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.

⚠️ We can't start reviewing your pull request until you've submitted a signed contributor agreement or indicated your institutional affiliation. Please see the CONTRIBUTING file for more information. If you've signed an agreement in the past, you may need to re-sign. See The New Home of the Open edX Codebase for details.

@ztraboo ztraboo force-pushed the ztraboo.maple.master/fix-frontend-app-learning-start branch from 1533494 to 268ddad Compare January 12, 2022 14:16
cmltaWt0
cmltaWt0 reviewed Jan 12, 2022
View reviewed changes
Copy link
Contributor

@cmltaWt0 cmltaWt0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally - everything is good for me.

repo.sh Outdated
@@ -33,11 +33,11 @@ repos=(
"https://github.com/edx/frontend-app-gradebook.git"
"https://github.com/edx/frontend-app-payment.git"
"https://github.com/edx/frontend-app-publisher.git"
"https://github.com/edx/frontend-app-learning.git"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super small comment - there is a trailing space at the end of the line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmltaWt0 I just removed that space. I added it because one of the test failed and I wanted the test to run again.

@natabene
Copy link

@ztraboo Thank you for the contribution! Please sign new version of our CLA (or have your employer sign a new version of the entity agreement), and we will be able to review your code.

@ztraboo
fix: setup frontend-app-learning to start by default.
ee4a403 
The documentation for `maple` mentions that the MFE for Learning is enabled by default.
https://edx.readthedocs.io/projects/open-edx-release-notes/en/latest/maple.html#learning-micro-frontend-mfe-becomes-the-default-courseware-experience

This was not the case when I setup my devstack install so this commit fixes that.
@ztraboo ztraboo force-pushed the ztraboo.maple.master/fix-frontend-app-learning-start branch from 268ddad to ee4a403 Compare January 12, 2022 16:06
@ztraboo ztraboo closed this Jan 12, 2022
@ztraboo ztraboo reopened this Jan 12, 2022
@openedx-webhooks
Copy link

@ztraboo Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@openedx-webhooks
Copy link

Hello @ztraboo: We are unable to continue with review of your submission at this time. Please see the associated JIRA ticket for more explanation.

@ztraboo
Copy link
Contributor Author

ztraboo commented Jan 12, 2022

@natabene I accidentally closed out then reopened this PR. Can you re-open this on your end? Also I waiting to get my Entity CLA agreement signed.

@cmltaWt0
Copy link
Contributor

@ztraboo I think it will be better to open two separate PRs.

  1. Backport of 19fdd17
  2. Moving learning app to release section.

In case CLA signing process delays backporting I can do it on my own.

@ztraboo
Copy link
Contributor Author

ztraboo commented Jan 12, 2022

@cmltaWt0 Sounds good. I'll make those two separate PR that you mention now.

This was referenced Jan 12, 2022
Merged
Merged
@ztraboo
Copy link
Contributor Author

ztraboo commented Jan 12, 2022

@cmltaWt0 See #881 and #882 for these two separate PR.

@ztraboo ztraboo mentioned this pull request Jan 12, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cmltaWt0 cmltaWt0 cmltaWt0 left review comments

Assignees
No one assigned
Labels
open-source-contribution PR author is not from Axim or 2U rejected
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ztraboo @openedx-webhooks @natabene @cmltaWt0 @pshiu