Embed: refactor view #7955
Merged
Embed: refactor view #7955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Make them more readable by splitting the html from the json file, similarly to what we do with the search tests. - Don't do two calls to storage, just open the file. - Remove unused requests adaptor (probably was something used before storage) Didn't bother to split the mkdocs json file, since that mkdocs doesn't output that kind of format anymore, we should start reading from the html file itself.
Co-authored-by: Manuel Kaufmann <humitos@gmail.com>
stsewd
commented
Feb 24, 2021
|
|
||
| if not section and headers: | ||
| # If no section is sent, return the content of the first one | ||
| # TODO: This will always be the full page content, | ||
| # lets do something smarter here | ||
| section = list(headers[0].keys())[0].lower() | ||
|
|
||
| if section: |
There was a problem hiding this comment.
Here only this big if was removed and the query_obj changed, everything else is the same.
- Use version.is_sphinx_type to choose between mkdocs or sphinx - Avoid nested code - Check for multiple queries inside a loop instead of several if's - Clean links only to the end result node, not to the whole body
stsewd
force-pushed
the
refactor-embed-view
branch
from
7473fca
to
3494f5f
Compare
|
Need to rebase to master to have a better diff :D |
Merged
humitos
reviewed
Mar 1, 2021
| ) | ||
|
|
||
|
|
||
| class EmbedAPI(SettingsOverrideObject): | ||
| _default_class = EmbedAPIBase | ||
|
|
||
|
|
||
| def do_embed(project, version, doc, section=None, path=None, url=None): | ||
| def do_embed(*, project, version, doc=None, path=None, section=None, url=None): |
There was a problem hiding this comment.
We are changing the signature of the function here. doc was required previously and now it's not. Why it's not required anymore?
There was a problem hiding this comment.
doc is the pagename, that's sphinx specific (isn't required when the project is mkdocs type as you can see below), and isn't required to get the response if path and section are given.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #7947