Skip to content

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

Closed
wants to merge 4 commits into from
Closed

Conversation

mattsb42-aws
Copy link
Member

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:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@mattsb42-aws mattsb42-aws requested a review from a team January 8, 2020 02:56
@mattsb42-aws
Copy link
Member Author

...aaand the workflows aren't triggering... :/

@mattsb42-aws
Copy link
Member Author

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.

@scottarc
Copy link

scottarc commented Jan 8, 2020

Is that an issue with Github Actions itself, or how we're using it?

@mattsb42-aws
Copy link
Member Author

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.

@robin-aws
Copy link
Contributor

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?

@mattsb42-aws
Copy link
Member Author

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?

@robin-aws
Copy link
Contributor

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?

@mattsb42-aws
Copy link
Member Author

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.

@mattsb42-aws
Copy link
Member Author

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.

Copy link

@dougch dougch left a 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
Copy link

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 }}
Copy link

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 }}
Copy link

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?

@mattsb42-aws
Copy link
Member Author

this was taken care of with #142

@mattsb42-aws mattsb42-aws deleted the ga branch October 23, 2020 15:43
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.

4 participants