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

[🫒 backport] fix: use getConfig in order to support runtime configuration #287

Conversation

kdmccormick
Copy link
Member

Backport of #286. Please use that PR for details & discussion.

@arbrandes
Copy link
Contributor

Why does that test fail here, but not in master? Weird.

@kdmccormick
Copy link
Member Author

@arbrandes only because I haven't updated this PR with Ghassan's fix yet 😛

@kdmccormick kdmccormick force-pushed the open-release/olive.master branch from f92c391 to 30c5b2f Compare November 28, 2022 14:55
@kdmccormick
Copy link
Member Author

kdmccormick commented Nov 28, 2022

Pushed updated commits, npm test should pass now...

@kdmccormick kdmccormick force-pushed the open-release/olive.master branch from 30c5b2f to 94a5ec4 Compare November 28, 2022 14:57
@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

❗ No coverage uploaded for pull request base (open-release/olive.master@bccd87f). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@                      Coverage Diff                      @@
##             open-release/olive.master      #287   +/-   ##
=============================================================
  Coverage                             ?   100.00%           
=============================================================
  Files                                ?       109           
  Lines                                ?      1264           
  Branches                             ?       248           
=============================================================
  Hits                                 ?      1264           
  Misses                               ?         0           
  Partials                             ?         0           

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.

…#286)

Before, gradebook was reading config from `process.env`
directly, which locked the app into using only
static (build-time) configuration. In order to
enable dynamic (runtime) configuration, we update
gradebook to use frontend-platform's standard
configuration interface: `mergeConfig()` and `getConfig()`.

Bumps version from 1.5.0 to 1.6.0.
(I would normally just do a patch release for a fix, but the version
 was hasn't been bumped for a while, so adding in full runtime
 configuration support seemed like it warranted a proper
 minor version bump.)

Co-authored-by: Ghassan Maslamani <ghassan.maslamani@gmail.com>
@kdmccormick kdmccormick force-pushed the open-release/olive.master branch from 94a5ec4 to bdfb2a6 Compare November 28, 2022 16:18
@kdmccormick kdmccormick marked this pull request as ready for review November 28, 2022 16:18
@kdmccormick kdmccormick requested a review from a team as a code owner November 28, 2022 16:18
@kdmccormick
Copy link
Member Author

@arbrandes Thanks for the review on the master PR! This one's ready for you too.

Copy link
Contributor

@muselesscreator muselesscreator left a comment

Choose a reason for hiding this comment

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

LGTM

@kdmccormick kdmccormick merged commit c4846f9 into openedx:open-release/olive.master Nov 28, 2022
@kdmccormick kdmccormick deleted the open-release/olive.master branch November 28, 2022 17:06
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.

3 participants