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

Move parser tests from rtd-build repo #4225

Merged
merged 13 commits into from
Jun 14, 2018

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jun 12, 2018

I'm not sure if I'm doing this in the proper way, hopefully it's a start.

@stsewd stsewd requested a review from agjohnson June 12, 2018 01:27
@agjohnson
Copy link
Contributor

👍 Yeah, so far so good. Is it easier to move this code over in parts like this, or to do all of readthedocs_build.config at once? If all at once, I'd say we can make doc_builder.config rely on the imported code with this PR as well.

@agjohnson agjohnson added this to the YAML File Completion milestone Jun 13, 2018
@agjohnson agjohnson added the PR: work in progress Pull request is not ready for full review label Jun 13, 2018
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

So far looking good. We can either merge this PR now or attach more parts of readthedocs_build.config to this PR before merge. I'll leave this up to @stsewd which approach to take.

@stsewd
Copy link
Member Author

stsewd commented Jun 13, 2018

Is it easier to move this code over in parts like this, or to do all of readthedocs_build.config at once?

I think is fine like this. But I'll attach another little part here (find.py)

Copy link
Member Author

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

I move this file, and the encoding problem was gone. I think the issue was the file having a bad encoding when created. See readthedocs/readthedocs-build#45 and #3732 (comment)

Probably the file was corrupted,
the test pass with a proper unicode filename.
See readthedocs#3732 (comment)
@stsewd
Copy link
Member Author

stsewd commented Jun 13, 2018

This is done, I'm leaving conf.py and validation.py for other PR. After travis is done and a re-review we can merge this :)

@stsewd stsewd requested a review from agjohnson June 13, 2018 21:14
We are using unicode literals
@stsewd
Copy link
Member Author

stsewd commented Jun 13, 2018

I forgot about the unicode literals 😞 in the previous commit. Fixed

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

LGTM!

@agjohnson agjohnson merged commit aafd455 into readthedocs:master Jun 14, 2018
@stsewd stsewd deleted the move-back-rtd-build branch June 14, 2018 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: work in progress Pull request is not ready for full review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants