Skip to content
This repository was archived by the owner on Apr 24, 2023. It is now read-only.

Replace Travis with Github Actions #150

Merged
merged 5 commits into from
Mar 14, 2022
Merged

Replace Travis with Github Actions #150

merged 5 commits into from
Mar 14, 2022

Conversation

vogdb
Copy link
Contributor

@vogdb vogdb commented Mar 2, 2022

I see that js-libp2p-tcp also has automerge.yml. Should I add it here as well?

@vogdb vogdb changed the title add test-and-release github action Replace Travis with Github Actions Mar 2, 2022
vogdb
vogdb commented Mar 3, 2022
View reviewed changes
.github/workflows/js-test-and-release.yml
with:
node-version: ${{ matrix.node }}
- uses: ipfs/aegir/actions/cache-node-modules@master
- run: npm install -g node-pre-gyp
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added installation of node-pre-gyp here. Let me know if I should move it elsewhere.

Copy link
Member

@vasco-santos vasco-santos Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@vogdb It looks like this needs to run after actions/setup-node@v2

See https://github.com/libp2p/js-libp2p-webrtc-direct/runs/5537246479?check_suite_focus=true

Don't now why PR CI did not fail though

Copy link
Contributor Author

@vogdb vogdb Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Please look at #152. Did I understand you correctly?
Btw should be Github Actions enabled somehow to start working on PRs?

@vogdb
Copy link
Contributor Author

vogdb commented Mar 3, 2022 •
edited
Loading

Well, I've added automerge.yml in the end. Another question from me. js-test-and-release.yml is a typical github action that is used in the most of libp2p repos. It comes from https://github.com/protocol/.github/. Now, it expects the project to have npm scripts: test-node, test-chrome, test-chrome-webworker, test-firefox, test-firefox-webworker, test-electron-main, test-electron-renderer. This project does not have them. Instead it has only test-node, test-browser. Should I keep only those two scripts in js-test-and-release.yml?

@vogdb vogdb mentioned this pull request Mar 3, 2022
Merged
wemeetagain
wemeetagain suggested changes Mar 4, 2022
View reviewed changes
.github/workflows/js-test-and-release.yml
with:
docker-token: ${{ secrets.DOCKER_TOKEN }}
docker-username: ${{ secrets.DOCKER_USERNAME }}
- run: npm run --if-present release
Copy link
Member

Choose a reason for hiding this comment

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

I think we would want the release process to be updated as well.
Take a look at the package.json from other libp2p repos, we'd want it to match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you mean integrating the semantic-release then I wanted to do it in a separate PR to keep things separate and clean. Currently working on migrating to Typescript. If you can, please answer my questions above.

@wemeetagain
Copy link
Member

Should I keep only those two scripts in js-test-and-release.yml?

I think you can leave the unused scripts since they're called with --if-present and seemingly won't break if they don't exist.

@vogdb
Copy link
Contributor Author

vogdb commented Mar 7, 2022

Thank you for clarifying. I will add test-browser then as it's missing in CI but present in package.json

@vogdb vogdb requested a review from wemeetagain March 9, 2022 09:01
@vogdb
Copy link
Contributor Author

vogdb commented Mar 14, 2022

I can't merge so I will cc @vasco-santos

vasco-santos
vasco-santos approved these changes Mar 14, 2022
View reviewed changes
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot @vogdb

@vasco-santos vasco-santos merged commit a73735b into libp2p:master Mar 14, 2022
@vogdb vogdb deleted the github-actions branch March 14, 2022 13:44
github-actions bot pushed a commit that referenced this pull request Mar 28, 2022
@semantic-release-bot
chore(release): 1.0.0 [skip ci]
ec7c5c1 
## [1.0.0](v0.7.1...v1.0.0) (2022-03-28)

### ⚠ BREAKING CHANGES

* this module is now ESM-only

Co-authored-by: achingbrain <[email protected]>

### Features

* convert to typescript ([#151](#151)) ([85ce5cf](85ce5cf))

### Bug Fixes

* add 'node-pre-gyp' installation to 'check' and 'test-node' actions ([#152](#152)) ([bf4a68b](bf4a68b))

### Trivial Changes

* add note about `node-pre-gyp` to readme.md ([#141](#141)) ([ab4cc82](ab4cc82)), closes [#140](#140)
* **deps-dev:** bump aegir from 35.2.1 to 36.0.0 ([#139](#139)) ([720cfad](720cfad))
* replace Travis with Github Actions ([#150](#150)) ([a73735b](a73735b))
* update project config ([13ab340](13ab340))
* update Readme ([#148](#148)) ([ba9facb](ba9facb))
@github-actions
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vasco-santos vasco-santos vasco-santos approved these changes

@wemeetagain wemeetagain wemeetagain approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vogdb @wemeetagain @vasco-santos