Add GitHub Action to build the project #601

D4nte · 2020-04-21T10:29:40Z

Ready for review, you can see the runs at https://github.com/D4nte/rust-lightning/actions

Same setup than Travis except for removing
rm -f target/debug/lightning-* as I do not believe
such file would exist on a fresh run.

I have not setup caching at this stage. The library is
small so I don't think it'd be that necessary/helpful.

I'd recommend to let both CI run for a bit to compare
performance and stability. The CI setup is straightforward
so I do not foresee any issue with GitHub actions.

Once happy, Travis file can be removed and branch
protection checks can be updated to block on the GitHub
actions.

You can also check the Coverage report to ensure it is as expected.

TheBlueMatt · 2020-04-21T17:17:34Z

Cool! Obviously needs fuzz targets etc but happy to move over.

valentinewallace · 2020-04-23T15:16:57Z

Hi, thanks much for this PR. I'm wondering, what are the advantages you were thinking that GitHub actions has over Travis?

I sort of see the advantages of using both but want to understand the argument for switching better :)

valentinewallace · 2020-04-23T17:35:36Z

.github/workflows/build.yml

+      - name: Build on Rust ${{ matrix.toolchain }}
+        if: ${{ matrix.build-net-tokio }} != true
+        run: RUSTFLAGS="-C link-dead-code" cargo build --verbose -p lightning


I could be mistaken, but it looks like this step is still running on rust-stable, where it shouldn't be? And the Test on Rust steps also seem to be running redundantly.
https://i.imgur.com/C0HYKL8.png (from your link to the runs in the PR description)

Also seems to be the case for 1.39.0

Yep, I totally misunderstood the if: syntax. Should look good now. Thanks.

valentinewallace

This is looking good, just a few outstanding comments :)

I wish the actions would run on this PR but it looks like that would only happen if you were a Collaborator: https://i.imgur.com/ChW4gVi.png

valentinewallace · 2020-04-23T20:57:53Z

.github/workflows/build.yml

+          - toolchain: stable
+            build-net-tokio: true
+          - toolchain: beta
+            build-net-tokio-tokio: true


Beta doesn't seem to be building net-tokio: https://i.imgur.com/wvQvKP6.png

Typo now fixed.

valentinewallace · 2020-04-23T21:02:28Z

.github/workflows/build.yml

+      - name: Build on Rust ${{ matrix.toolchain }}
+        if: ${{ matrix.build-net-tokio }} != true
+        run: RUSTFLAGS="-C link-dead-code" cargo build --verbose -p lightning


Also seems to be the case for 1.39.0

valentinewallace · 2020-04-23T21:21:18Z

.github/workflows/build.yml

@@ -0,0 +1,93 @@
+name: Continuous Integration Checks


Not a dealbreaker but, wondering if there's a way to get the colors that Travis has, in github actions output? They're nice

D4nte · 2020-04-24T00:02:17Z

Hi, thanks much for this PR. I'm wondering, what are the advantages you were thinking that GitHub actions has over Travis?

I sort of see the advantages of using both but want to understand the argument for switching better :)

Hi @valentinewallace, @TheBlueMatt mentioned some instability on Travis that is why I did the PR 😁
In term of switching or keeping both, it's really up to you (maintainers) as you'll have a better idea of what would work best.

TheBlueMatt · 2020-04-24T00:18:00Z

I've found travis to be pretty flaky since the acquisition, and it seems more and more like its on maintinence-mode, they fired a bunch of folks at the acquisition IIRC and a bunch of the images etc hanve't been updated since :/.

TheBlueMatt · 2020-04-24T02:29:56Z

How do we make this run? The "Checks" tab shows no checks here? I dont see any options in the repo settings to turn it on, or does it need to be on master first?

Same setup than Travis except for removing `rm -f target/debug/lightning-*` as I do not believe such file would exist on a fresh run. I have not setup caching at this stage. The library is small so I don't think it'd be that necessary/helpful. I'd recommend to let both CI run for a bit to compare performance and stability. The CI setup is straightforward so I do not foresee any issue with GitHub actions. Once happy, Travis file can be removed and branch protection checks can be updated to block on the GitHub actions. You can also check the [Coverage report](https://codecov.io/gh/D4nte/rust-lightning/tree/752a58bc0441a49a0513f2cad979ad9e2621312a/lightning/src/chain) to ensure it is as expected.

D4nte · 2020-04-24T04:56:55Z

How do we make this run? The "Checks" tab shows no checks here? I dont see any options in the repo settings to turn it on, or does it need to be on master first?

As @valentinewallace mentioned, because I am not a collaborator and I am using a fork, the new workflow I bring in does not get triggered on rust-bitcoin/rust-lightning repo. This is to protect any secrets you may have setup for GitHub Action on this repo, stopping non-maintainers to reveal them by echo-ing them with new workflow/PRs.

However, the actions are triggered on my fork, so you can see the result at https://github.com/D4nte/rust-lightning/actions for the same branch.

Once this is in master, I expect the checks to be triggered for any new PR, fork or not.

valentinewallace

Looks good now! :) Thanks for the updates (+ quick turnaround).

D4nte force-pushed the ci-in-github-action branch 4 times, most recently from 682e133 to 752a58b Compare April 22, 2020 23:15

Use more generic name as it will be use in GitHub Action too

e6e69f5

D4nte force-pushed the ci-in-github-action branch from 752a58b to 1585e8d Compare April 22, 2020 23:36

D4nte marked this pull request as ready for review April 22, 2020 23:38

valentinewallace self-requested a review April 23, 2020 14:54

valentinewallace reviewed Apr 23, 2020

View reviewed changes

D4nte force-pushed the ci-in-github-action branch from ff3d157 to 79f9965 Compare April 24, 2020 04:04

D4nte force-pushed the ci-in-github-action branch from b984f32 to 355dbbc Compare April 24, 2020 04:49

Add a pop of colour

06419a2

valentinewallace approved these changes Apr 24, 2020

View reviewed changes

TheBlueMatt merged commit 86a2607 into lightningdevkit:master Apr 24, 2020

D4nte deleted the ci-in-github-action branch April 27, 2020 00:51

Add GitHub Action to build the project #601

Add GitHub Action to build the project #601

Uh oh!

Conversation

D4nte commented Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Apr 21, 2020

Uh oh!

valentinewallace commented Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valentinewallace Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valentinewallace Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

D4nte Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

valentinewallace left a comment

Choose a reason for hiding this comment

Uh oh!

valentinewallace Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

D4nte Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

valentinewallace Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

valentinewallace Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

D4nte Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

D4nte commented Apr 24, 2020

Uh oh!

TheBlueMatt commented Apr 24, 2020

Uh oh!

TheBlueMatt commented Apr 24, 2020

Uh oh!

D4nte commented Apr 24, 2020

Uh oh!

valentinewallace left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

D4nte commented Apr 21, 2020 •

edited

Loading

valentinewallace commented Apr 23, 2020 •

edited

Loading

valentinewallace Apr 23, 2020 •

edited

Loading

valentinewallace left a comment •

edited

Loading