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: allowing customization of notes repo and version #19

Merged
merged 3 commits into from
Feb 28, 2023

Conversation

jfavellar90
Copy link
Collaborator

@jfavellar90 jfavellar90 commented Feb 15, 2023

This PR adds 2 new configurations for the plugins:

NOTES_REPOSITORY: (default: "https://github.com/openedx/edx-notes-api")
NOTES_REPOSITORY_VERSION: (default: "{{ OPENEDX_COMMON_VERSION }}")

These will allow setting custom notes code repository and notes code version to build the image, which enables adding custom features for notes service on top of the default upstream version.

@jfavellar90 jfavellar90 requested a review from regisb February 15, 2023 13:14
README.rst Outdated
@@ -33,6 +33,8 @@ Configuration
- ``NOTES_HOST`` (default: ``"notes.{{ LMS_HOST }}"``)
- ``NOTES_MYSQL_DATABASE`` (default: ``"notes"``)
- ``NOTES_MYSQL_USERNAME`` (default: ``"notes"``)
- ``NOTES_CODE_REPO`` (default: ``"https://github.com/edx/edx-notes-api"``)
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be consistent with Tutor core, these new settings should be named NOTES_REPOSITORY and NOTES_VERSION.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@regisb using NOTES_VERSION will cause a clash with the already existing version variable which refers to the Tutor plugin version. Should I use something like NOTES_COMMON_VERSION?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point. Can we use "NOTES_REPOSITORY_VERSION" instead please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@regisb addressed. I think this is ready to go

tutornotes/plugin.py Outdated Show resolved Hide resolved
@jfavellar90 jfavellar90 force-pushed the Jhony/custom_code_and_version branch 3 times, most recently from c94ecf9 to 76269f2 Compare February 24, 2023 14:14
@jfavellar90 jfavellar90 force-pushed the Jhony/custom_code_and_version branch from 76269f2 to 069e262 Compare February 27, 2023 17:44
@jfavellar90 jfavellar90 force-pushed the Jhony/custom_code_and_version branch from 069e262 to 600895c Compare February 27, 2023 17:48
@regisb regisb merged commit 527c3e9 into master Feb 28, 2023
@regisb regisb deleted the Jhony/custom_code_and_version branch February 28, 2023 08:36
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.

2 participants