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

Migrate CI workflows from Travis CI to GitHub Actions #108

Merged
merged 2 commits into from Jul 9, 2020

Conversation

hectcastro
Copy link
Contributor

@hectcastro hectcastro commented Jul 8, 2020

Migrate the existing Travis CI testing workflow to GitHub Actions. The following baseline success criteria has been addressed:

  • Trigger linters, code formatters, and test suites on pull requests
  • Ensure that the builds test against Python version 3.6-3.8
  • Cache dependencies across builds
  • Trigger PyPI releases of Python 3.x packages on tagged releases

Fixes #103
Fixes #104


Testing

  • Review the GitHub Actions build output for this pull request
  • Review the GitHub Actions release workflow output (This was expected to fail with a "File already exists" error.)
  • Ensure that test executes cleanly
$ ./scripts/test

@hectcastro hectcastro marked this pull request as ready for review July 8, 2020 19:04
@hectcastro hectcastro requested a review from lossyrob July 8, 2020 19:04
.gitignore Outdated Show resolved Hide resolved
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.6", "3.7", "3.8"]
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 dropped Python 3.5 from here and the package metadata because it goes end-of-life on 2020-09-13. Do you think that's going to cause any significant problems?

See: https://devguide.python.org/#status-of-python-branches

Copy link
Member

@lossyrob lossyrob Jul 8, 2020

Choose a reason for hiding this comment

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

I don't foresee problems there.

.gitignore Outdated Show resolved Hide resolved
@@ -179,7 +179,7 @@ def clear_children(self):
Return:
Catalog: Returns ``self``
"""
self.links = [l for l in self.links if l.rel != 'child']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the Python specific changes below are to resolve outstanding linter warnings.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting that the linter is picking these up now as opposed to before through local and travis. Wonder if this rule was introduced in a later version? I have flake8 3.7.8 and 0.28.0 on my machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After looking at this closer, it looks like this was caused by a mistake on my part. I was following the guidance of my editor, which was using a newer version of flake8 (3.8.3).

Most of the issues were related to E741. I think two reasonable paths forward are:

  1. Bump flake8 in requirements-dev.txt to 3.8+ and keep the adjustments in.
  2. Revert the adjustments and keep the dependencies as-is.

I'm in favor of #1 because it seems like a reasonable role, and updating flake8 makes sense. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, #1 makes sense, would be best upgrade and the lint changes are improvements.

pystac/serialization/identify.py Show resolved Hide resolved

set -e

if [[ -n "${CI}" ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is set in the GitHub Actions execution environment by default.


- name: Build and publish package
env:
TWINE_USERNAME: ${{ secrets.PYPI_AZAVEA_USERNAME }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These PyPI credentials are pulled from the GitHub organizational level (azavea). I also swapped ownership within PyPI from rasterfoundry -> azavea. The rasterfoundry PyPI user is still listed as a maintainer.

Copy link
Member

Choose a reason for hiding this comment

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

Nice, thanks.

README.md Show resolved Hide resolved
Copy link
Member

@lossyrob lossyrob left a comment

Choose a reason for hiding this comment

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

Pulled this down and ran through scripts/test. Looked at the GitHub action outputs. Everything looks good.

I made two comments on small non-critical changes that can be addressed either in this PR in follow ups.

@hectcastro hectcastro requested a review from lossyrob July 9, 2020 11:32
Migrate the existing Travis CI testing workflow to GitHub Actions,
by achieving the following success criteria:

- Trigger linters, code formatters, and test suites on pull requests
- Ensure that the builds test against Python version 3.6-3.8
- Cache dependencies across builds
- Trigger PyPI releases of Python 3.x packages on tagged releases
Copy link
Member

@lossyrob lossyrob left a comment

Choose a reason for hiding this comment

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

Tested flake8 upgrade and unit test change locally. 👍

@hectcastro hectcastro merged commit 18adff4 into develop Jul 9, 2020
@hectcastro hectcastro deleted the feature/hmc/github-actions branch July 9, 2020 15:54
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.

Migrate from Travis CI to GitHub Actions Travis failing to publish tag to pypi
2 participants