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

fix: enable meilisearch in MFE config #231

Merged

Conversation

pomegranited
Copy link

@Faraz32123
Copy link

why not adding this setting to openedx-lms-common-settings instead of adding in both dev and prod patches?

@regisb
Copy link
Contributor

regisb commented Oct 31, 2024

It's true we could probably move the shared configurations to "openedx-lms-common-settings". I'd say it's slightly beyond the scope of this PR, and we risk running into rebase conflicts if we make other changes in the master branch until Sumac is released. But I'll agree with whatever ya'll decide.

@pomegranited
Copy link
Author

@Faraz32123 Let me know what you'd like me to do? I'm just a user of this repo, not an expert like y'all :)

@Faraz32123
Copy link

@Faraz32123 Let me know what you'd like me to do? I'm just a user of this repo, not an expert like y'all :)

@pomegranited all good from your side, Thanks. I think, we can merge this PR.
We'll refactor the patches in a separate PR.

Copy link
Contributor

@DawoudSheraz DawoudSheraz left a comment

Choose a reason for hiding this comment

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

One thing: please add a changelog entry and it should be ready to merge.

@DawoudSheraz
Copy link
Contributor

I am going to merge this PR, we can add the changelog in a follow-up PR. Since Sumac sandbox is going to be re-created on Monday morning, it is essential to have this change in place as tutor core PR for meilisearch has been merged.

@DawoudSheraz DawoudSheraz merged commit 47b76a8 into overhangio:sumac Nov 1, 2024
@pomegranited pomegranited deleted the jill/sumac/mfe-enable-meilisearch branch November 6, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants