Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Added GitHub Actions CI Workflow #261

Merged
merged 4 commits into from
Jan 29, 2020
Merged

Added GitHub Actions CI Workflow #261

merged 4 commits into from
Jan 29, 2020

Conversation

bmiddha
Copy link
Member

@bmiddha bmiddha commented Jan 3, 2020

Added a GitHub Actions Workflow with a matrix strategy for Node.js 8.x, 10.x, and 12.x and across Ubuntu, MacOS, and Windows with support for node_modules caching to improve speed.

@orta
Copy link
Contributor

orta commented Jan 15, 2020

I'm happy with this 👍

My opinion is that it might not need to be so comprehensive (testing on Mac/window/linux) across 2 node versions? Worth bringing it down to just linux?

@peterblazejewicz
Copy link
Collaborator

Worth bringing it down to just linux?
And just lastest Node LTS (12)

@orta
Copy link
Contributor

orta commented Jan 15, 2020

haha, guess we're in alignment then :D

@bmiddha
Copy link
Member Author

bmiddha commented Jan 16, 2020

I wanted to add an OS build matrix to make sure the contributors get alerted by build fails if they attempt to do some OS specific commands in their code / config files. (for example using rm or del in the scripts section of package.json to clean built files.)
Should I continue to remove it?
Also, should I remove the travis.yml or keep both Travis CI and GitHub Actions for CI testing?
@orta @peterblazejewicz

path: ${{ steps.npm-cache.outputs.dir }}
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-node-
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that OK, when ending with dash?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ofc, sorry for missing this 🙇

Copy link
Collaborator

@peterblazejewicz peterblazejewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, LGTM
@orta

@orta
Copy link
Contributor

orta commented Jan 29, 2020

Yep 👍

@orta orta merged commit 68d6cf5 into microsoft:master Jan 29, 2020
brittanydrandolph pushed a commit that referenced this pull request Jun 17, 2022
Added GitHub Actions CI Workflow
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants