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
Search: refactor json parser #7184
Conversation
- Use classes - Get the code ready to index directly form html - Remove duplicated tests - Rename some functions/variables Logic is the same
@@ -1,32 +0,0 @@ | |||
import os |
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.
We already have a more general tests in search/tests/test_parsers.py
readthedocs/projects/models.py
Outdated
parser_class = ( | ||
SphinxParser if self.version.is_sphinx_type else MkDocsParser | ||
) | ||
parser = parser_class(self.project, self.version) |
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.
We likely only need a version here, since it already relates to project.
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 thought we were doing that for performance
readthedocs.org/readthedocs/projects/models.py
Lines 1314 to 1319 in fb5b8c8
project = models.ForeignKey( | |
'Project', | |
verbose_name=_('Project'), | |
related_name='imported_files', | |
on_delete=models.CASCADE, | |
) |
But I think django will still do another query at least we are not doing a prefetch somewhere
- foo.rst | ||
- foo/index.rst | ||
|
||
Both lead to foo/index.html. |
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.
This is a good docstring
Logic is the same