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 CI test #63

Open
wants to merge 3 commits into
base: node10
Choose a base branch
from
Open

Conversation

haata
Copy link

@haata haata commented Nov 22, 2020

Very basic GitHub Actions configuration.

@kentonv
Copy link
Member

kentonv commented Nov 30, 2020

Thanks for writing this!

I wonder why the action didn't actually run on this PR itself? It would be nice to see that it passes before merging. I was under the impression that PRs were allowed to modify workflows, and those modifications would actually be used when running the workflow on the PR, so I don't get why it's not happening here.

I wonder if node.js is possibly not a valid action name, because it contains a .... I would name it just test.

@haata
Copy link
Author

haata commented Nov 30, 2020

Yeah, I'm not sure how the Actions stuff works for PRs. I think there's some sort of approval process to even enable actions (so you don't just get random PRs running code for actions, but I'm not too sure). The Actions work on my own fork, so I know they run https://github.com/haata/node-capnp/actions.

I still need to do some work on the Windows version of this, haven't been able to get it to compile yet. Will post a separate PR when it's ready.

@ocdtrekkie
Copy link
Contributor

I am thinking it'll only run on a PR where the branch name is node10...

Actions shouldn't need any permission to run, it's just that Actions on PRs can't access secrets, generally.

@ocdtrekkie
Copy link
Contributor

ocdtrekkie commented Nov 30, 2020

Sandstorm's is just on: [push, pull_request] and I think that probably would work fine here.

@haata
Copy link
Author

haata commented Dec 1, 2020

I made the change, still doesn't seem to show up here.
Again, works on my branch: https://github.com/haata/node-capnp/actions/runs/393176929

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants