-
Notifications
You must be signed in to change notification settings - Fork 56
add local tests and static analysis workflows using GitHub Actions #134
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
Conversation
...aaand the workflows aren't triggering... :/ |
Per offline research, I discovered that GA workflows do NOT trigger on the PR that creates the workflow file. These workflows will not trigger until a subsequent PR. |
Is that an issue with Github Actions itself, or how we're using it? |
Based on my testing, this is a fundamental characteristic of GitHub Actions. Presumably as a protection against random new workflows being added to repos without the maintainers' knowledge or permission. |
Should the workflow be to create a fork from a repo and get the existing Actions, plus whatever new or updated ones you're contributing, to work on your fork, so that you can safely demonstrate the changes? How likely is that to be feasible? |
That's the hard thing. I haven't figured what the best "priming" action is. Maybe an empty file, then fill it in a second PR? |
I admit I don’t quite see why only existing configured actions are live. Safety doesn’t seem to be a valid argument since malicious actors could just submit PR’s that mess with existing actions instead. Or are there other guard rails I’m not aware of? |
It's not a perfect mechanism, there might be other reasons, and my guess about why it works that way might be wrong. It's also not really relevant, though. It's not something we can do anything about, it's just how the tool works. |
side note: as I mentioned in #135, which is priming the repo so that these workflows will actually run in this PR, I've removed the branch requirement for Travis to pass because it looks like it's broken at the moment and I'd rather invest in moving to GitHub Actions than fixing Travis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Questions about matrix values
@@ -0,0 +1,50 @@ | |||
name: Local Tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name: Integ Tests
- uses: actions/setup-python@v1 | ||
with: | ||
python-version: ${{ matrix.python }} | ||
architecture: ${{ matrix.architecture }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this implicit? Shouldn't there be an architecture in the matrix?
- uses: actions/setup-python@v1 | ||
with: | ||
python-version: ${{ matrix.python }} | ||
architecture: ${{ matrix.architecture }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, where is this defined?
this was taken care of with #142 |
Description of changes:
Initial conversion to GitHub Actions.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: