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

Update tests failing on master #4575

Merged
merged 1 commit into from Aug 28, 2018
Merged

Update tests failing on master #4575

merged 1 commit into from Aug 28, 2018

Conversation

@stsewd
Copy link
Member

@stsewd stsewd commented Aug 28, 2018

Ref to #4461 (comment)

So, this was caused because the PR was outdated, makes me think if we could do better to prevent this. What about requiring the PRs to be updated before merging to master? Now GitHub supports a button to update the PR from the web, so the process isn't slow, but we will need to wait to the tests to pass again with the new merge, for simple PRs this can be a little overkill, but in general, I think this is a sane check

@agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Aug 28, 2018

Changes look good!

What part of the PR wasn't updated? Do you mean you had local commits ready and didn't push up to your branch on GitHub yet?

Loading

@stsewd
Copy link
Member Author

@stsewd stsewd commented Aug 28, 2018

The PR was done before #4456, so it was using the old interface. Then the new interface was merged, and the tests were keeping the old interface.

Loading

Copy link
Member

@humitos humitos left a comment

Thanks!

Loading

@humitos humitos merged commit dde104d into readthedocs:master Aug 28, 2018
1 check passed
Loading
@stsewd stsewd deleted the update-tests branch Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants