-
Notifications
You must be signed in to change notification settings - Fork 84
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
Fix: MFE_HOST check with the new CONFIG_LOADED action #150
Fix: MFE_HOST check with the new CONFIG_LOADED action #150
Conversation
This will make sure that MFE_HOST is a subdomain of LMS_HOST, and if not, it will print a warning to notify the user.
since the CONFIG_LOADED action was introduced in the 16.1.2, this version should not be installed in the older versions.
d8d1de0
to
8fdc5ec
Compare
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.
LGTM! Can you please just add a changelog entry with scriv create
?
Done. |
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.
lgtm!
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.
No objections, thanks!
As a part of overhangio/tutor#557, we should warn the users if they're not following the apps.LMS_HOST pattern for the MFE_HOST.
This will introduce a new check to see if the MFE_HOST is a subdomain of LMS_HOST or not using the new CONFIG_LOADED action .