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.

Sign up for GitHub

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

Jump to bottom

fix: [email protected] is not the right email for Open edX queries #201

Merged
merged 1 commit into from
Jun 14, 2023
Merged

fix: [email protected] is not the right email for Open edX queries #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion edx_api_doc_tools/conf_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

description="APIs for access to Open edX information",
):
"""
Expand Down
4 changes: 2 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ def is_requirement(line):
description="Tools for writing and generating API documentation for edX REST APIs",
long_description=README + '\n\n' + CHANGELOG,
long_description_content_type="text/x-rst",
author='edX',
author_email='oscm@edx.org',
author='Open edX Project',
author_email='oscm@openedx.org',
Comment on lines +113 to +114
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 👍🏻

url='https:/openedx/api-doc-tools',
packages=[
'edx_api_doc_tools',
Expand Down