GitHub Actions Improvements #437
Conversation
|
@epgts, this probably conflicts with some of the stuff you're doing, so I'm happy to wait until that's merged and try to resolve myself. |
rtwalker
force-pushed
the
rw/github-actions
branch
from
30517f9
to
47a26c7
Compare
|
@rtwalker Glad to see you taking the initiative to make it more maintainable. I noticed this when I was working on the other Github action. We also actually made the switch from 'master' to 'main' branch in a release branch IIRC. The change did not somehow made to the main branch. |
| @@ -3,7 +3,7 @@ on: | |||
| pull_request: | |||
| push: | |||
| branches: | |||
| - master | |||
| - main | |||
There was a problem hiding this comment.
Does this mean anything at all? We have no branched named 'master' and yet we had no problems. Should we delete this setting?
There was a problem hiding this comment.
I'm indifferent to whether we should have this set. This was just to get things working the way they were (at some point) intended to.
The "problem" this causes is just that the CI workflow doesn't run as often as intended. Where the workflow could have run every time a commit was pushed to main, it didn't because it was instead looking on master.
I think we have some branch protections on main that mean that every commit coming into main was brought in via PR, where we just ran the CI workflow, so I'm open to the argument that we no longer intend for things to work this way.
There was a problem hiding this comment.
@rtwalker Yup, you got it; this was set back when the branch was called master and before we hand branch protections. I think what bors does isn't a push, so it shouldn't matter what's here.
rtwalker
force-pushed
the
rw/github-actions
branch
from
47a26c7
to
96820cf
Compare
|
bors r+ |
437: GitHub Actions Improvements r=rtwalker a=rtwalker Adding a few commits of things I've thought might improve our GitHub Actions setup while I spent some time this week reading through the documentation. I think the changes are mostly superficial (except I caught one place we still have `master` instead of `main`), but thought I'd offer to contribute them nevertheless. Co-authored-by: Ryan Walker <rwalker@timescale.com>
|
Timed out. |
|
bors retry |
437: GitHub Actions Improvements r=rtwalker a=rtwalker Adding a few commits of things I've thought might improve our GitHub Actions setup while I spent some time this week reading through the documentation. I think the changes are mostly superficial (except I caught one place we still have `master` instead of `main`), but thought I'd offer to contribute them nevertheless. Co-authored-by: Ryan Walker <rwalker@timescale.com>
|
Timed out. |
|
bors retry |
437: GitHub Actions Improvements r=rtwalker a=rtwalker Adding a few commits of things I've thought might improve our GitHub Actions setup while I spent some time this week reading through the documentation. I think the changes are mostly superficial (except I caught one place we still have `master` instead of `main`), but thought I'd offer to contribute them nevertheless. Co-authored-by: Ryan Walker <rwalker@timescale.com>
|
Timed out. |
| # TODO Why? Cargo default is to pass `-C incremental` to rustc; why don't we want that? | ||
| # https://doc.rust-lang.org/rustc/codegen-options/index.html#incremental | ||
| # Well turning it off takes the extension target size down from 3G to 2G... |
There was a problem hiding this comment.
The theory is that enough tends to change between builds that incrementalism won't help (and frankly incremental has been buggy recently). There's even arguments to actively remove the current binary from the build cache to cut down even more (that's what rust-analyzer, who tend to be the experts on this, used to do, thought it seems like they've since changed to rust-cache)
| # TODO What reads this? It's not listed on | ||
| # https://doc.rust-lang.org/cargo/reference/environment-variables.html | ||
| CI: 1 | ||
| RUST_BACKTRACE: short | ||
| # TODO We don't seem to run rustup, nor does it seem like we should. |
There was a problem hiding this comment.
I believe these are currently unused
Using the matrix "strategy" with GitHub actions means that we don't have 3 jobs that are nearly identical and de-duplicates a lot
Our ci.yml file is getting long, thought this might help make our workflows easier to read. Also added some paths-ignore, since there are some cases where there is no need to run clippy.
rtwalker
force-pushed
the
rw/github-actions
branch
from
96820cf
to
ea52aca
Compare
|
bors retry |
|
Build succeeded: |
Adding a few commits of things I've thought might improve our GitHub Actions setup while I spent some time this week reading through the documentation. I think the changes are mostly superficial (except I caught one place we still have
masterinstead ofmain), but thought I'd offer to contribute them nevertheless.