GitHub actions #1635
GitHub actions #1635
Conversation
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/markedjs/markedjs/5z6k4mrtb |
|
Since travis hasn't been working on PRs it seems like a good time to switch to github actions. |
| - name: Checkout Code | ||
| uses: actions/checkout@v2 | ||
| - name: Install Node | ||
| uses: dcodeIO/setup-node-nvm@master |
There was a problem hiding this comment.
I think I would rather use the official node.js action and run tests on mac and windows as well as linux. Is this only used to reference lts?
There was a problem hiding this comment.
Is this only used to reference lts?
Yes, there is no way to reference lts or latest with the official node.js action. And updating the version number will be forgotten if we don't automate it.
There was a problem hiding this comment.
and run tests on mac and window
I don't really see how this library can change depending on which os it is on. It has no dependencies and no os dependent code.
If it does run differently on different os's with node that would be a node issue not a marked issue.
There was a problem hiding this comment.
I suppose maybe testing the cli but we don't do that right now anyway.
| node-version: ${{ matrix.node_version }} | ||
| - name: Install Dependencies | ||
| run: npm ci | ||
| - name: Run Unit Tests 👩🏽💻 |
There was a problem hiding this comment.
Should we run lint before unit tests instead of a separate job? If it fails lint then it could fail fast.
There was a problem hiding this comment.
I think it would be better to run in parallel. We could turn off the fail-fast so it will always run both tests on node and lts and lint. For that matter we could also run unit tests and spec tests in parallel too if we wanted.
|
I updated the required checks to use the GH Actions instead of Travis so we unblock other PRs 👍 |
|
I had to remove the checks for the protected master branch in order to allow GitHub Actions to push after the build step. We might need to use a different token if we want those checks. |
|
That seems like a bad side effect. It looks like you removed required reviewers too 😬 |
|
ya. I don't really see that as too much of a problem since there aren't very many people who can merge PRs and we all agree there should be a reviewer before merging. |
|
Not to mention we could all bypass that requirement before if we really wanted to. |
It's more about the process and having it codeified into the platform. I get hundreds of github notifications for reviews across hundreds of repos so its always good to have as much help from the system as possible so I don't forget when to merge and when to wait. |
GitHub actions
Description
Use github actions
Contributor
Committer
In most cases, this should be a different person than the contributor.