[Merged by Bors] - Deprecate travis CI #2165
Conversation
in addition to Travis
Update docs and comments
|
I don't think GA has a 10 minute inactivity timeout like Travis does, so actually, these patches should probably be removed entirely. Going to try to get this merged sooner rather than later because nothing is merging correctly right now, but I'll make sure to open an issue to follow up on that too. |
There was a problem hiding this comment.
Funny about the "travis patch"... That solution actually didn't work... When tests are normally executed logs aren't printed unless a test fails. The solution ended up being running the app test separately from other tests with --verbose. This is actually good for other reasons as well: the test runs concurrently with the rest of the app tests, making the suite complete faster and we get real-time logs from this very long test, which is nice.
I approved the PR, my comments can be addressed later.
| @@ -16,6 +16,7 @@ | |||
| [](https://goreportcard.com/report/github.com/spacemeshos/go-spacemesh) | |||
| [](https://app.bors.tech/repositories/22421) | |||
| <a href="https://godoc.org/github.com/spacemeshos/go-spacemesh"><img src="https://img.shields.io/badge/godoc-LGTM-blue.svg"/></a> | |||
| [](https://github.com/spacemeshos/go-spacemesh/actions) | |||
There was a problem hiding this comment.
Not sure we should add a "CI passing" badge (assuming this is what this line does). The way bors works is that the build on develop cannot possibly be broken (nothing that breaks develop gets merged).
Do you disagree, @lrettig ?
There was a problem hiding this comment.
You're right, of course, but my thought was that, to the uninitiated observer who doesn't know what bors is, it's nice to see that we're using CI and that our CI build is passing :) It's sort of something I tend to look for when surfing projects and repositories!
What do you think we should do?
There was a problem hiding this comment.
I initially removed the CI badge and replaced it with the bors badge, hoping that it would be a good replacement (it links to an explanation of what bors is). But having read that a "CI passing" badge is something you look for (this makes sense), I've re-evaluated my position and now I think we should keep it (even though it logically makes no sense).
I wish bors had a badge that statically says "Build passing | guaranteed by bors" or even just "BORS | passing" instead of "BORS | enabled"...
There was a problem hiding this comment.
Good point. Maybe we can change what the bors badge says 🤔
There was a problem hiding this comment.
this is the image provided by bors:
https://bors.tech/images/badge_small.svg ->
Of course we could create whatever image we want (badges are just images, sometimes the image served is selected based on status, but not in bors' case).
| "systemtest-latenodes", | ||
| "systemtest-blocks-add-node", | ||
| "systemtest-hare-mining", | ||
| "systemtest-sync-blocks", | ||
| "systemtest-genesis-p2p", |
There was a problem hiding this comment.
I'm still not liking this - can't we somehow say that all CI tests must pass without specifying them individually?
There was a problem hiding this comment.
As I wrote earlier, I can't think of an easy way to do this. Based on the bors documentation and bors-ng/bors-ng#730, the bors.toml file needs a "List of commit statuses" that matches what GitHub displays as "Required statuses" (see below 👇). If we want the tests to all run in parallel, each needs to be an independent "job" (in the lingo of Workflow syntax for GitHub Actions) which means it'll get a separate line under "Required statuses."
I can only think of two ways to do this. 1. Hack bors itself, ideally so that it gets the list of "Required statuses" itself using the GitHub API, or 2. write a Makefile script that auto-generates bors.toml somehow, although as I indicated earlier it might still take some complicated finagling to get bors to read a dynamically-generated config file. I suggest that we leave this as a separate issue and a task for later - it would be a great, self-contained task for a community contribution.
There was a problem hiding this comment.
Opened #2172 to follow up
|
bors merge |
## Motivation We recently added support for github actions (#2060). Right now we're running both GA and travis at the same time, and as a result we're seeing more timeouts in our tests than usual (see, e.g., #2164). ## Changes - Turns off travis CI - Removes all references to travis in comments and docs - Tells bors to stop looking at travis status ## Test Plan N/A ## To Do - [ ] Open separate issue to remove "travis timeout" patches
|
Build failed:
|
|
bors merge |
## Motivation We recently added support for github actions (#2060). Right now we're running both GA and travis at the same time, and as a result we're seeing more timeouts in our tests than usual (see, e.g., #2164). ## Changes - Turns off travis CI - Removes all references to travis in comments and docs - Tells bors to stop looking at travis status ## Test Plan N/A ## To Do - [ ] Open separate issue to remove "travis timeout" patches
|
Pull request successfully merged into develop. Build succeeded: |
Motivation
We recently added support for github actions (#2060). Right now we're running both GA and travis at the same time, and as a result we're seeing more timeouts in our tests than usual (see, e.g., #2164).
Changes
Test Plan
N/A
To Do