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

feat: Support runtime configuration (second attempt) #386

Merged
merged 1 commit into from
Dec 20, 2022

Conversation

arbrandes
Copy link
Contributor

(This reintroduces the change in 9f84230 that was later reverted by 67b0b33.)

frontend-platform supports runtime configuration since 2.5.0 (see the PR that introduced it[1], but it requires MFE cooperation. This implements just that: by avoiding making configuration values constant, it should now be possible to change them after initialization.

Almost all changes here relate to the LMS_BASE_URL setting, which in most places was treated as a constant.

[1] openedx/frontend-platform#335

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Base: 84.71% // Head: 84.68% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (db8984e) compared to base (b1c1f1c).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head db8984e differs from pull request most recent head b0cac75. Consider uploading reports for the commit b0cac75 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #386      +/-   ##
==========================================
- Coverage   84.71%   84.68%   -0.03%     
==========================================
  Files         134      134              
  Lines        2761     2756       -5     
  Branches      768      768              
==========================================
- Hits         2339     2334       -5     
  Misses        400      400              
  Partials       22       22              
Impacted Files Coverage Δ
src/components/NavigationBar/data/api.js 30.00% <100.00%> (ø)
src/data/api.js 100.00% <100.00%> (ø)
src/data/constants.js 100.00% <100.00%> (ø)
src/discussions/cohorts/data/api.js 100.00% <100.00%> (ø)
src/discussions/comments/data/api.js 91.66% <100.00%> (-0.34%) ⬇️
src/discussions/data/api.js 100.00% <100.00%> (ø)
src/discussions/learners/data/api.js 100.00% <100.00%> (ø)
src/discussions/posts/data/api.js 91.66% <100.00%> (-0.23%) ⬇️
src/discussions/topics/data/api.js 85.71% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@asadazam93
Copy link
Contributor

@arbrandes have you updated anything in this PR? OR do you want to just merge and try to debug on stage?

@arbrandes
Copy link
Contributor Author

As noted on the parent issue, no changes here yet. Going to do another visual pass.

(This reintroduces the change in 9f84230 that was later reverted by
67b0b33.)

frontend-platform supports runtime configuration since 2.5.0 (see the PR
that introduced it[1], but it requires MFE cooperation.  This implements
just that: by avoiding making configuration values constant, it should
now be possible to change them after initialization.

Almost all changes here relate to the `LMS_BASE_URL` setting, which in
most places was treated as a constant.

[1] openedx/frontend-platform#335
@mehaknasir mehaknasir merged commit 0c71e8b into openedx:master Dec 20, 2022
moonesque pushed a commit to edSPIRIT/frontend-app-discussions that referenced this pull request Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants