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: [email protected] is not the right email for Open edX queries #201

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

nedbat
Copy link
Contributor

@nedbat nedbat commented May 22, 2023

No description provided.

@nedbat nedbat requested a review from kdmccormick May 22, 2023 14:07
@nedbat
Copy link
Contributor Author

nedbat commented Jun 7, 2023

@kdmccormick ping?

Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

Sorry for missing your first ping!

@@ -41,7 +41,7 @@ def make_docs_urls(api_info, api_url_patterns=None):
def make_api_info(
title="Open edX APIs",
version="v1",
email="oscm@edx.org",
email="oscm@openedx.org",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is the right email either -- an Open edX instance running the default configuration shouldn't be directing mail to our OSCM :)

Thing is, I can't think of any other sensible default. How about making this a required argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A required argument that who will supply? And where?

Copy link
Member

Choose a reason for hiding this comment

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

The caller of this function, which is generally a Django project with access to Django settings, would supply it. I believe that's what edx-platform and registrar do currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my experience, there's very few emails because of this address (6/year?). I'm hoping to get this changed so that Axim is the point of contact rather than 2U, without having to change the calling signature. This function is called without an email argument from a number of places:

edx/demographics/demographics/urls.py
edx/edx-exams/edx_exams/urls.py
edx/portal-designer/designer/urls.py
edx/video-encode-manager/encode_manager/urls.py
openedx/blockstore/blockstore/urls.py
openedx/course-discovery/course_discovery/urls.py
openedx/enterprise-access/enterprise_access/urls.py
openedx/enterprise-catalog/enterprise_catalog/urls.py
openedx/license-manager/license_manager/urls.py

Changing this argument will be a breaking change, bumping the version number to 2.0.0, which will get upgraded to automatically, and these repos will have to change their calls.

What would you like to do?

Copy link
Member

Choose a reason for hiding this comment

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

Wow, I didn't realize this library was installed into so many services. I guess we must have put it in the cookiecutter at some point.

[email protected] is good enough. I guess we can change it later if we get too many confused emails.

Comment on lines +113 to +114
author='Open edX Project',
author_email='oscm@openedx.org',
Copy link
Member

Choose a reason for hiding this comment

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

I think these look good, let me double-check with the team.

Copy link
Member

Choose a reason for hiding this comment

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

Team agrees that these are good values 👍🏻

@nedbat nedbat merged commit 7c859d3 into master Jun 14, 2023
@nedbat nedbat deleted the nedbat/no-oscm-edx-email branch June 14, 2023 22:26
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