Skip to content

Implement mkdocs key from v2 config #4486

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

Merged
merged 16 commits into from
Sep 27, 2018
Merged

Implement mkdocs key from v2 config #4486

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c1d791e
Implement mkdocs key
stsewd Aug 7, 2018
5918ce0
Add mkdocs key to v1
stsewd Aug 7, 2018
019fc27
Unskip tests
stsewd Sep 10, 2018
8b077e9
Use config object in python env
stsewd Sep 11, 2018
050c974
Move auto detection for doctype
stsewd Sep 11, 2018
c469d64
Use config object to get doctype
stsewd Sep 11, 2018
ce16fe7
Remove mock
stsewd Sep 11, 2018
9caace0
Revert changes on documentation_type/doctype
stsewd Sep 17, 2018
d9d2f25
Fix test
stsewd Sep 17, 2018
b0d23a6
Validate same doctype (web and config file)
stsewd Sep 17, 2018
3962715
Use relative path to shown error
stsewd Sep 18, 2018
1aa8126
Better error message
stsewd Sep 19, 2018
ca60dca
Add reason on skipped tests
stsewd Sep 19, 2018
96f6241
Better error message
stsewd Sep 20, 2018
a03da2b
Better error message
stsewd Sep 21, 2018
357f034
Better error msg
stsewd Sep 26, 2018
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
10 changes: 7 additions & 3 deletions readthedocs/config/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,12 +823,16 @@ def validate_final_doc_type(self):
should support per version doctype, but we need to
adapt the rtd code for that.
"""
if self.doctype != self.defaults.get('doctype', 'sphinx'):
dashboard_doctype = self.defaults.get('doctype', 'sphinx')
if self.doctype != dashboard_doctype:
key = 'mkdocs' if self.doctype == 'mkdocs' else 'sphinx'
self.error(
key,
'Your documentation type should match '
'the one from the admin panel of your project.',
'Your project is configured as "{doctype}" in your admin '
Copy link
Contributor

Choose a reason for hiding this comment

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

doctype here is the interpolation that could be displayed as sphinx_htmldir. key in this string can only be sphinx or mkdocs.

'dashboard, but there was a "{key}" key specified.'.format(
Copy link
Contributor

Choose a reason for hiding this comment

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

One last nitpick. The error is sort of inverted i feel -- we don't actually tell the user the they need a "sphinx" key, just that they can't have a "mkdocs" key. I feel we need to be clearer here, ie:

Your project is configured as "Sphinx" in your project admin dashboard, but there is no "sphinx" key specified.

Also, for some of the odd documentation types, it looks like we're putting the slug in there, so:

Your project is configured as "sphinx_htmldir" ...

I feel like we want the verbose name instead. This shouldn't be a huge change, just using the model field to resolve the verbose name, but if it is a large change, it's not horribly important. This piece is temporary and it's probably close enough for most users.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmm, I'm not sure about accessing the db from the config module (we don't do that there). I'm changing the error message.

doctype=dashboard_doctype,
key=key,
),
code=INVALID_KEYS_COMBINATION,
)

Expand Down
3 changes: 2 additions & 1 deletion readthedocs/config/tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,8 @@ def test_sphinx_builder_default(self):
build.validate()
build.sphinx.builder == 'sphinx'

@pytest.mark.skip
@pytest.mark.skip(
'This test is not compatible with the new validation around doctype.')
def test_sphinx_builder_ignores_default(self):
build = self.get_build_config(
{},
Expand Down
3 changes: 2 additions & 1 deletion readthedocs/rtd_tests/tests/test_config_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,8 @@ def test_sphinx_builder(

get_builder_class.assert_called_with(result)

@pytest.mark.skip
@pytest.mark.skip(
'This test is not compatible with the new validation around doctype.')
@patch('readthedocs.projects.tasks.get_builder_class')
def test_sphinx_builder_default(
self, get_builder_class, checkout_path, tmpdir):
Expand Down