This ports the CI to use Github Actions on pull requests and pushes. Unfortunately it looks like you have to merge this before it takes effect. You can see the runs on my repository at https://github.com/kallewoof/bips/actions though.
For posterity, I initially did a port to Cirrus before it was pointed out that Github Actions may be better.
Nit: Having all the checks in one job means that if the first check fails, you don’t know if the 2nd or 3rd one fails, so sometimes you get a few extra back and forth “nope, try again.” and can be annoying as a maintainer.
Other than that, ACK.
@junderw That’s not the case with GitHub actions… You can drilldown and see exactly which step of the job failed, along with debug/verbose info written to the console by the build script:
e.g.
https://github.com/augustoproiete-forks/bitcoin--bips/actions/runs/4361581418/jobs/7625593905
exit 1…? Either way, splitting into multiple sounds good to me but felt slightly overkill maybe.
We had a similar decision between Cirrus and Github Actions for rust-bitcoin. My understanding is that Core chose to eschew Github Actions because of an abundance of caution around Github’s security controls (which IIRC had been in the news recently), and in general, Github being the canonical source of truth for the repo so there’s the potential for security issues with GA to be much more serious.
What we landed on was “this is probably reasonable paranoia for the reference implementation of Bitcoin’s consensus logic, but probably not worth the extra hassle for other libraries, since GA is so convenient”. I’d say the BIPs repo probably has the same calculus. It’d be pretty surreal if there was a security issue in GA that led to a BIP text being changed, then somehow nobody noticing for the months it took for implementations to appear and be deployed.
@kallewoof @junderw Oh, I see now that I misread @junderw’s comment - I thought he was talking about the ability to see which step failed, and not about continuing the execution when one of the steps fail.
It seems moving the execution of link-format-chk.sh to its own job would be a good idea, if buildtable.pl can be run independently (and not fail - even if links are not in the expected format)
“this is probably reasonable paranoia for the reference implementation of Bitcoin’s consensus logic, but probably not worth the extra hassle for other libraries, since GA is so convenient”
Exactly. GA is perfectly fine for BIPs repo imo.
Also, I could have been more explicit.
By running in parallel, all tests are run. But in sequence, if the first one fails, the 2nd and 3rd one are not run so you only learn their status after fixing the first one and trying again. If parallel, you can see that all 3 are failing and fix all 3 in one commit.
@junderw So like https://github.com/bitcoin/bips/pull/1432/commits/685f383f8155357fe1f822fe6f50b0c649aa888a ? (e.g. https://github.com/kallewoof/bips/actions/runs/4370385560 )
Looks a bit redundant, but it did indeed run the tests in parallel.
