Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

build: updating to sphinx<6.0.0 constraint. #199

Merged
merged 2 commits into from
Jan 16, 2023
Merged

Conversation

awais786
Copy link
Contributor

@awais786 awais786 commented Jan 16, 2023

Fix Sphinx version constraint found details here

Upgraded to sphinx==5.3.0

6.0.0 is not compatible with this theme thats why added a <6.0.0 constraint here.

In older versions they were ignoring first arg. But in new version they stopped ignoring. Thats why updated the command build_main(['-b', 'html', '.', build_path])

For details check the following PR
https://github.com/sphinx-doc/sphinx/pull/3668/files

@codecov
Copy link

codecov bot commented Jan 16, 2023

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (d8d423a) compared to base (6e06ac2).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head d8d423a differs from pull request most recent head 314bf5a. Consider uploading reports for the commit 314bf5a to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #199   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           19        19           
=========================================
  Hits            19        19           
Impacted Files Coverage Δ
edx_theme/__init__.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -18,7 +18,7 @@ def html_dir():
docs_path = os.path.join(os.path.abspath(os.path.dirname(__file__)), '..', 'docs')
build_path = os.path.join(docs_path, '_test', 'html')
os.chdir(docs_path)
result = build_main(['sphinx-build', '-b', 'html', '.', build_path])
result = build_main(['-b', 'html', '.', build_path])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 older version it strips the first arg.

-> return build.build_main(argv[1:])  # skip first argument to adjust arguments (refs: #4615)
(Pdb) argv
['sphinx-build', '-b', 'html', '.', '/Users/awaisqureshi/Documents/devstack/edx-sphinx-theme/tests/../docs/_test/html']
(Pdb) argv[1:]
['-b', 'html', '.', '/Users/awaisqureshi/Documents/devstack/edx-sphinx-theme/tests/../docs/_test/html']

Copy link
Contributor Author

@awais786 awais786 Jan 16, 2023

Choose a reason for hiding this comment

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

In latest version first arg is not ignoring thats why it throws the error due to 'sphinx-build'

> /Users/awaisqureshi/.pyenv/versions/3.8.3/lib/python3.8/site-packages/sphinx/cmd/build.py(270)build_main()
-> args = _parse_arguments(argv)
(Pdb) sys.argv[1:]
[]
(Pdb) argv
['sphinx-build', '-b', 'html', '.', '/Users/awaisqureshi/Documents/devstack/edx-sphinx-theme/tests/../docs/_test/html']
(Pdb) n

@awais786 awais786 marked this pull request as ready for review January 16, 2023 17:13
@awais786 awais786 changed the title build: updating to sphinx<6.0.0 constraint. 6.0.0 is not compatible w… build: updating to sphinx<6.0.0 constraint. Jan 16, 2023
@@ -15,7 +15,7 @@ jobs:
matrix:
os: [ubuntu-20.04]
python-version: ['3.8']
toxenv: [sphinx11, sphinx14, docs, quality]
toxenv: [sphinx50, docs, quality]
Copy link
Contributor

@mumarkhan999 mumarkhan999 Jan 16, 2023

Choose a reason for hiding this comment

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

tox.ini Outdated
@@ -34,8 +34,7 @@ norecursedirs = .* docs requirements

[testenv]
deps =
sphinx18: Sphinx==1.1.3
django19: Sphinx>=1.4,<1.5
Sphinx50: Sphinx>=5.0.0,<6.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

snowballstemmer==2.2.0
# via sphinx
sphinx==1.8.5
sphinx==5.3.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

verified

Copy link
Contributor

@mumarkhan999 mumarkhan999 left a comment

Choose a reason for hiding this comment

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

  • 👍 LGTM
  • 🔔 squash before the merge.

@awais786 awais786 merged commit 3763bed into master Jan 16, 2023
@awais786 awais786 deleted the upgrade-sphnix- branch January 16, 2023 20:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants