-
Notifications
You must be signed in to change notification settings - Fork 3
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
config(course): add title update license fields #292
config(course): add title update license fields #292
Conversation
This looks good to me. Note that there is another request for changes to these field though: https://github.com/mitodl/hq/issues/4065 I don't know if one PR is easier than two, but I expect that putting them into the same release would be simpler. |
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.
I don't think that this PR should be modifying RELEASE.rst
and VERSION
; that's probably an unintentional change.
Should be simple enough to add it to this PR, seems okay since we need it by tomorrow.
pre-commit ci bot added those changes. RELEASE.rst seems to be a one-time change (adds a line break at the end). We might need to exclude VERSION in the pre-commit config. |
60a275f
to
00cf3d2
Compare
@pdpinch, I've added the license options as requested in https://github.com/mitodl/hq/issues/4065. I've also added @pt2302, I've excluded the files you mentioned from pre-commit. The pre-commit check is failing, probably because of an unrelated reason. I'll ask DevOps to look at it. |
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.
Looks good to me!
@pdpinch, do the changes seem okay to you too? |
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.
👍
you can take or leave my comment
@@ -214,6 +214,8 @@ collections: | |||
value: https://creativecommons.org/licenses/by/4.0/ | |||
- label: CC BY-NC | |||
value: https://creativecommons.org/licenses/by-nc/4.0/ | |||
- label: CC BY-SA | |||
value: https://creativecommons.org/licenses/by-sa/4.0/ |
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.
nit: Please sort the CC variants alphabetically:
CC BY
CC BY-NC
CC BY-NC-SA
CC BY-SA
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/4065
Description (What does it do?)
This PR adds a help text to the title. The title is a required field and is not required to be defined, but it has to be defined to add the help text.
This PR also adds a new license option
BY CC-SA
and changes default for external resources as requested in https://github.com/mitodl/hq/issues/4065.Screenshots (if appropriate):
How can this be tested?
hussaintaj/external-resource-title-help-text
in your local clone ofocw-hugo-projects
.ocw-studio
project.ocw-studio/localdev/configs/default-course-config.yml
with the content ofocw-hugo-projects/ocw-course-v2/ocw-studio.yaml
.docker compose exec web ./manage.py override_site_config -c localdev/configs/default-course-config.yml -s ocw-course-v2
External Resources
tab.BY CC-SA
.All Rights Reserved
.Add Link
button in the CKEditor UI.