feat(ci): use github actions instead of travis-ci

[sudoforge]

No description provided.

[sudoforge]

cc @ashkulz

[sudoforge]

@trygveaa I'd recommend requiring these checks to pass after merging the PR.

[sudoforge]

Added two new commits which add python 3.8 and 3.9 to the test matrix. Please be sure to use any merge strategy except squashing (or squashing via interactive rebasing) so that the logical boundary of the commits remains.

[sudoforge]

I'm also sitting on two commits:

• adding black
• formatting all the source files with black and adding a step in the lint job to verify all files are formatted correctly

but i think these would be best suited for adding in a separate PR

[trygveaa]

Could you add notification to #wee-slack-dev on freenode when the build fails, like the travis config has?

[sudoforge]

Could you add notification to #wee-slack-dev on freenode when the build fails, like the travis config has?

Some initial cursory research doesn't show that this is possible. Github Actions currently supports email and web-based notifications for failed runs. I'll do a little more research tonight and/or think about how to best add support for this.

Would this be considered a blocking feature? If this ends up requiring fairly heavy time involvement to support, I don't know that I'll have the time within the next 1-2 months to add support for it.

[trygveaa]

Alright, thanks for having a look. Lets drop it. (irc notifications that is, not github actions).

[sudoforge]

A user has created an action which interacts with IRC: https://github.com/rectalogic/notify-irc

But the problem with using this comes about by the way GitHub Actions works; if a step fails then the workflow short circuits and reports a failure (via the email and/or web notification system, depending on your user settings).

It might be possible to set some parameter to allow a step to fail but continue (I haven't had the need to look for this option yet, personally); I'll look into this tonight and get back to you by tomorrow morning.

It may be helpful to note that if a workflow fails, the user that triggered the event(s) causing the workflow to run is the user that is notified. I don't believe you'd be notified for a run failure due to my PR, for example, but I would.

[trygveaa]

Don't worry, let's drop it, you don't need to spend more time on it. I'm the only one in the IRC channel anyways, and I can use email notifications instead. I don't think I want an overly complex GitHub Actions config, the config is already pretty large.

It may be helpful to note that if a workflow fails, the user that triggered the event(s) causing the workflow to run is the user that is notified. I don't believe you'd be notified for a run failure due to my PR, for example, but I would.

Good to know, but for PRs from others I see the failure right above the merge button, so that's fine.

[sudoforge]

Good to know, but for PRs from others I see the failure right above the merge button, so that's fine.

Right, it definitely isn't obfuscated or hard to find failed runs :)

You'll also see the green check mark (incdicating success) or red "x" (indicating failure) on the PR overview page, e.g. https://github.com/wee-slack/wee-slack/pulls

[sudoforge]

Oops, accidentally hit the "close and comment" button.

---

FWIW, I've contacted GitHub about the dual run caused by internal branches used to create PRs. If they get back to me with a better solution than the conditional check I have here on each step, I'll update the tree (or submit a new PR if this one is merged).

[sudoforge]

I concluded the GitHub Support ticket. The conditional statement I have here on each step is the only way to accomplish the goal (to only run the workflow in response to only one of push and pull_request events.

[ashkulz]

@sudoforge I'm not sure we need that condition; imagine that I'm the maintainer and make a PR just to get CI feedback -- it won't help as I think your condition will block the PR checks. Secondly, imagine that such a PR gets out-of-date due to other PRs getting merged -- having the checks being run again is good, as they'll be run again on the merge commit or rebased/squashed commit (which is not the same as the original commit).

[sudoforge]

@ashkulz you're misunderstanding how GitHub Actions works, the on triggers, and the conditional check.

Given the current configuration, the workflow will run:

• On any commit/update to any branch on the wee-slack/wee-slack repository. (due to the on: push trigger)
• On any commit/update to any PR for any external repository (due to the on: pull_request trigger and the conditional)

Because the push event will run the workflow for any branch in this repository, we want to prevent these so-called "internal branches" from dual triggering when they are part of a PR. That's the purpose of the conditional. An internal branch that is used in a PR will still run on every commit/update due to the push event.

Without the conditional, internal branches used in a PR would run the workflow twice (on each update): once for the push events, once for the pull_request events.

[sudoforge]

To put it another way...

If you look at the checks that have been reported for this PR, you'll see the suffix (pull_request) at the end. This is fine, because this is a PR created from an external repository (mine).

If this PR's HEAD branch was from the wee-slack/wee-slack repository, and we didn't have the conditional, we'd see 16 checks reported (8 from the push events, 8 from the pull_request events), With the conditional, we'd see 8 successful (from push events) and 8 skipped (pull_request events).

[ashkulz]

To give an actual example, see this PR I made for testing in another repository -- the workflow definition is similiar but there were no runs for the branch, I just saw the PR runs. You can verify this by going to the last commit as well.

So the solution might be to just use branches: [ master ] for the push configuration like the above does, instead of this weird configuration?

[sudoforge]

That would make the workflow only run for push events to master, and not for other branches in the repository.

[sudoforge]

The current configuration is the only way to:

• Have each commit in the repository run the workflow, and
• Have all PRs run the workflow, and
• Ensure no tree runs the workflow twice for each event

[ashkulz]

That would make the workflow only run for push events to master, and not for other branches in the repository.

I believe that's a trade-off that would work for most repos, but yeah you're right -- if the requirements are as you've stated, then there's no other solution 🤷‍♂️

[sudoforge]

Yeah, if the maintainers collectively determine that they're fine with only running the workflow on commits to master and PRs (excluding commits to branches made in this repository that aren't submitted as a PR), then we can simplify the workflow by being more explicit with the on specification.

[trygveaa]

Thanks!

[trygveaa]

I'm also sitting on two commits:

• adding black
• formatting all the source files with black and adding a step in the lint job to verify all files are formatted correctly

but i think these would be best suited for adding in a separate PR

I've also considered using black, so feel free to open a PR for that.