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

Fixes #2592 #2593

Merged
merged 1 commit into from Dec 11, 2016

Conversation

Projects
None yet
2 participants
@felixfontein
Copy link
Contributor

commented Dec 10, 2016

Forgot one return...

@felixfontein felixfontein requested a review from Kwpolska Dec 10, 2016

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2016

@Kwpolska: the failing tests are the baseline. I don't see what went wrong here, though; maybe some dependency changed which now generates a slightly different output? Also, the same failing baseline differences show up for #2594, which has completely different code changes.

@Kwpolska Kwpolska merged commit 5bd54e1 into master Dec 11, 2016

3 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
codacy/pr Good work! A positive pull request.
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 11, 2016

That’s a lxml update, I just rebuilt the baseline.

@Kwpolska Kwpolska deleted the fixing-2592 branch Dec 11, 2016

@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 11, 2016

PS. small and obvious fixes do not need to be done through a pull request, you may commit to master directly.

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2016

Nope, that doesn't work anymore. I tried that first, and I got an error message that a review is required when trying to push to master. So I had to convert it to a PR first.

@Kwpolska

This comment has been minimized.

Copy link
Member

commented Dec 11, 2016

Oh, that’s a side effect of requiring reviews and protecting branches. Which does not affect administrators, so I didn’t notice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.