Skip to content
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

Add feature to build json with html in the same build #4229

Merged
merged 4 commits into from
Jun 13, 2018

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jun 12, 2018

Replacement of #4213 Depends on readthedocs/readthedocs-sphinx-ext#43 Closes #3393

I need to add some tests and test manually.

def move(self, **__):
super(HtmlBuilder, self).move()
if self.project.has_feature(Feature.BUILD_JSON_ARTIFACTS_WITH_HTML):
# Copy json artifacts to its own directory
Copy link
Member Author

Choose a reason for hiding this comment

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

As the original SearchBuilder isn't executed, we need a move the assets to the other location, I'm not sure if here is the best place, but I feel like it belongs here (since the json output now comes from the html build)

Copy link
Member

Choose a reason for hiding this comment

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

Seems as good a place as any.


# Features
'generate_json_artifacts': self.project.has_feature(
Feature.BUILD_JSON_ARTIFACTS_WITH_HTML
Copy link
Member Author

Choose a reason for hiding this comment

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

This is injected in the configuration file

@@ -1015,13 +1015,16 @@ def add_features(sender, **kwargs):
ALLOW_DEPRECATED_WEBHOOKS = 'allow_deprecated_webhooks'
PIP_ALWAYS_UPGRADE = 'pip_always_upgrade'
SKIP_SUBMODULES = 'skip_submodules'
BUILD_JSON_ARTIFACTS_WITH_HTML = 'build_json_artifacts_with_html'
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if the name is the best

@stsewd
Copy link
Member Author

stsewd commented Jun 13, 2018

I just need to figure out how the write tests for this, but it works locally :).

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

I like how simple this change is! Looks like a great way to get this tested.

def move(self, **__):
super(HtmlBuilder, self).move()
if self.project.has_feature(Feature.BUILD_JSON_ARTIFACTS_WITH_HTML):
# Copy json artifacts to its own directory
Copy link
Member

Choose a reason for hiding this comment

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

Seems as good a place as any.

@ericholscher ericholscher merged commit 4a21c71 into readthedocs:master Jun 13, 2018
@stsewd stsewd deleted the use-feature-to-build-json branch June 13, 2018 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants