Skip to content

Conversation

@jrbourbeau
Copy link
Member

This remove an empty file that was committed (xref #4279 (comment))

@jakirkham
Copy link
Member

Thanks James! 😄

I restarted CI to see if that clears things up, but we may need to rebased this on latest master to get the pip fix in commit ( 1c86be3 ). For context some CIs (like Travis CI, Azure, etc.), create a merge commit between the PR and the base branch the PR is targeting, master in this case. However some CIs (like CircleCI) don't do this. As a result, sometimes users need to explicitly perform this merge with the base branch itself.

I'm not sure where GitHub Actions fits between these two. If it is in the former group, then this shouldn't be needed. Though if it is in the later, we may need to merge or rebase to ensure we have the fix. Based on a couple of PRs yesterday, I think manual action may be needed, but could be wrong.

@jrbourbeau
Copy link
Member Author

jrbourbeau commented Dec 4, 2020

Thanks for the additional context and CI bump @jakirkham!

I had merged the master branch locally (which contained changes from #4310) before creating a branch for this PR. Looking at my local git history for this branch I get:

(distributed) ➜  distributed git:(remove-test-file) ✗ git log --pretty=format:"%h - %an : %s" | head -n 5
65558264 - James Bourbeau : Remove empty test_highgraph.py file
7ea9696c - jakirkham : Coerce new `TaskState.nbytes` value to `int` (#4311)
3407aa31 - James Bourbeau : Remove offload try/except for thread_name_prefix keyword (#4308)
1c86be32 - jakirkham : Fix `pip` install issue on CI (#4310)
dcdb4651 - Simon Perkins : Transmit Layer annotations to scheduler (#4279)

So IIUC I would have expected the fix from 1c86be3 to be included regardless of if GitHub actions creates a merge commit with the base branch or not

@jakirkham
Copy link
Member

Interesting, well it doesn't look like we are seeing the issue on GitHub Actions any more. Not sure why that one job still had issues.

Happy to merge now. I'm guessing we don't wait for Travis CI any more due to the slow queue and credit limit issues. Is that right?

@jrbourbeau jrbourbeau merged commit c89e9ba into dask:master Dec 4, 2020
@jrbourbeau jrbourbeau deleted the remove-test-file branch December 4, 2020 18:42
@jrbourbeau
Copy link
Member Author

Yeah, for this PR in particular I think it's safe to not wait for Travis : )

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