-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Fixing make docs issue #25799
Fixing make docs issue #25799
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,8 @@ | |
'contentstore.apps.ContentstoreConfig', | ||
'cms.djangoapps.course_creators', | ||
'xblock_config.apps.XBlockConfig', | ||
'user_tasks', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Defined in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, Dave added that just 2 weeks ago in https://github.com/edx/edx-platform/pull/25933 . |
||
'lms.djangoapps.lti_provider' | ||
]) | ||
|
||
|
||
COMMON_TEST_DATA_ROOT = '' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise undefined var error appeared. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -255,6 +255,17 @@ | |
modules[path] = path | ||
|
||
|
||
# These Django apps under lms don't import correctly with the "lms.djangapps" prefix | ||
# Others don't import correctly without it...INSTALLED_APPS entries are inconsistent | ||
lms_djangoapps = ['badges', 'branding', 'bulk_email', 'courseware', | ||
'coursewarehistoryextended', 'email_marketing', 'experiments', 'lti_provider', | ||
'mobile_api', 'notes', 'rss_proxy', 'shoppingcart', 'survey'] | ||
for app in lms_djangoapps: | ||
path = os.path.join('lms', 'djangoapps', app) | ||
if app not in ['notes']: | ||
modules[path] = path | ||
|
||
|
||
def update_settings_module(service='lms'): | ||
""" | ||
Set the "DJANGO_SETTINGS_MODULE" environment variable appropriately | ||
|
@@ -283,6 +294,7 @@ def on_init(app): # pylint: disable=unused-argument | |
exclude_dirs = ['envs', 'migrations', 'test', 'tests'] | ||
exclude_dirs.extend(cms_djangoapps) | ||
exclude_dirs.extend(lms_djangoapps) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect we can rip out all this out pretty soon, but probably best to wait until the sys.path hack removal is fully complete. |
||
|
||
exclude_files = ['admin.py', 'test.py', 'testing.py', 'tests.py', 'testutils.py', 'wsgi.py'] | ||
for module in modules: | ||
module_path = six.text_type(root / module) | ||
|
@@ -315,5 +327,5 @@ def on_init(app): # pylint: disable=unused-argument | |
|
||
def setup(app): | ||
"""Sphinx extension: run sphinx-apidoc.""" | ||
event = b'builder-inited' if six.PY2 else 'builder-inited' | ||
event = 'builder-inited' | ||
app.connect(event, on_init) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -136,3 +136,6 @@ sympy==1.6.2 | |
|
||
# cryptography 3.3.1 started failing tests on sandbox because it dropped support for python3.5 | ||
cryptography==3.2.1 | ||
|
||
# greater versions breaking the code | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please write a ticket summarizing the problem with newer versions and link to it from here? Also, it looks like the docs requirements file hasn't been updated accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, its due to conflicts. |
||
Sphinx==3.3.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.
I think this should be harmless, but why was it needed?
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.
it was giving error during make docs command. I will add the complete traceback
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.
@jmbowman Otherwise this error appears