Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add github actions #135

Closed
wants to merge 1 commit into from

Conversation

@ryanio
Copy link
Contributor

ryanio commented Dec 2, 2019

Adds Github Actions workflows:

Node 8, 10, 12, 13: test
Node 12: lint, coveralls, coverage

You can see how the checks will look here.

I also put together a sample circleci config in another branch for comparison: config.yml and result.


Closes #133

@holgerd77

This comment has been minimized.

Copy link
Member

holgerd77 commented Dec 2, 2019

This actually looks really nice (GitHub Actions reporting) and is a lot better integrated, wonder if we should switch here? Any thoughts @s1na @alcuadrado @evertonfraga?

@ryanio

This comment has been minimized.

Copy link
Contributor Author

ryanio commented Dec 2, 2019

Yes I'm also new to Github Actions but it's really nice so far.

A separate question, what are the differences between tslint and lint, should we run both on CI? Travis was just running lint.

How about coverage vs coveralls vs nyc? Travis was just running coveralls, but I should be able to easily add all of them as separate checks.

@evertonfraga

This comment has been minimized.

Copy link
Member

evertonfraga commented Dec 2, 2019

coveralls: is a service to track and report coverage changes (notice the badge on repo readme)
nyc: tool from istanbul that collects coverage while tests are run
npm run coverage: script that runs nyc

@evertonfraga

This comment has been minimized.

Copy link
Member

evertonfraga commented Dec 2, 2019

Something that helps me quickly inspect all projects differences is to inspect their scripts section. Here's how you can print all of them:

cat ethereumjs-*/package.json | jq '{name: .name, script: .scripts}'

requires that each project preserves the ethereumjs- prefix in their directory names, and jq.

@evertonfraga

This comment has been minimized.

Copy link
Member

evertonfraga commented Dec 2, 2019

@holgerd77 Definitely looks nicer, as it's more integrated and organized.

Ryan's idea and effort came in super handy, as I've been thinking about the CI strategy for cross-library tests and monorepo.

I will RTM and see if it compares to CircleCI in some specific aspects. I'll then assess if it's worth to drop the workflows (build tree) back to a flat job structure.

@alcuadrado

This comment has been minimized.

Copy link
Member

alcuadrado commented Dec 3, 2019

This looks really good! Am I reading the results incorrectly, or are actions much faster than travis? travis jobs took about 3min, and actions ~1.

@ryanio

This comment has been minimized.

Copy link
Contributor Author

ryanio commented Dec 3, 2019

This looks really good! Am I reading the results incorrectly, or are actions much faster than travis? travis jobs took about 3min, and actions ~1.

It seems to be consistently that fast!

@ryanio ryanio changed the title test: github actions Add github actions Dec 4, 2019
@holgerd77

This comment has been minimized.

Copy link
Member

holgerd77 commented Dec 4, 2019

@ryanio The tslint command is running the linter together with the formatter (prettier), see scripts here, I agree that it's a bit confusing combination and naming.

@evertonfraga evertonfraga mentioned this pull request Dec 4, 2019
0 of 8 tasks complete
@ryanio ryanio force-pushed the ryanio:github-actions branch from 6c584ab to f2f2e69 Dec 5, 2019
@ryanio ryanio force-pushed the ryanio:github-actions branch from f2f2e69 to 43b9059 Dec 5, 2019
@ryanio ryanio closed this Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.