Skip to content

Comments

BLD: circleCI- merge before build, add -n to sphinx#17485

Merged
charris merged 4 commits intonumpy:masterfrom
mattip:circleci1
Oct 8, 2020
Merged

BLD: circleCI- merge before build, add -n to sphinx#17485
charris merged 4 commits intonumpy:masterfrom
mattip:circleci1

Conversation

@mattip
Copy link
Member

@mattip mattip commented Oct 7, 2020

Add a merge to the checkout step as per this comment to merge master into the PR.

Also add a step to show how many warnings are still being emitted with sphinx-build -n ..., xref issue gh-13114. It should not cause the build to fail, but those who are interested can dive into the log files and see the progress.

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@mattip
Copy link
Member Author

mattip commented Oct 7, 2020

The added job works, but I am not sure the merge-with-head post step is working in the checkout step

@eric-wieser
Copy link
Member

The downside of this PR is that it increases the doc build time. Does it make sense to do the nitpick checks after building and uploading the doc artifacts, to make this extra delay only matter to those fixing warnings? Or should we leave it as is to fix all of the warnings?

@mattip
Copy link
Member Author

mattip commented Oct 7, 2020

The downside of this PR is that it increases the doc build time.

I view this as a "temporary" step till we get the warning count down (but that could take a while). The CircleCI build time is 26 minutes (single process). Travis is running for ~55 minutes wall clock, 2hrs50 min process time, so I think the extra ~5 minutes are not going to slow down the PR turn-around time.

@eric-wieser
Copy link
Member

so I think the extra ~5 minutes are not going to slow down the PR turn-around time.

Note that for doc-only PRs, the travis build time doesn't really matter. Still, 5 minutes over 25 is not a disaster.

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Let's put this in and see if we can knock down the number of warnings

@charris charris merged commit 8d9a048 into numpy:master Oct 8, 2020
@charris
Copy link
Member

charris commented Oct 8, 2020

Thanks Matti. The big drain on the testing seems to be PRs with a dozen small updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants