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

Activate GH actions. #7

Merged
merged 13 commits into from
Nov 17, 2020
Merged

Activate GH actions. #7

merged 13 commits into from
Nov 17, 2020

Conversation

icemac
Copy link
Member

@icemac icemac commented Nov 13, 2020

Activate GH actions with the following features:

  • have names for the jobs
  • have separate jobs for lint and coverage
  • cache pip files
  • run the tests weekly on linux

@icemac icemac marked this pull request as ready for review November 13, 2020 16:30
Copy link
Member

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

LGTM.

There's a Travis badge in README.rst. It should be replaced with some GHA badge. I'm sure those exist, as I've seen one in https://eli.thegreenplace.net/2020/github-actions-first-impressions/ (but that page embeds the SVG instead of linking to it).

.github/workflows/tests.yml Outdated Show resolved Hide resolved
- name: Test
run: tox -e {{ matrix.env || 'py'}}
run: tox -e ${{ matrix.config[2] }}
Copy link
Member

Choose a reason for hiding this comment

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

The config matrix seems very repetetive, which gives me an idea. Could we do matrix.config[2] || 'py' here and fill the last column with null or "" or perhaps even omit it entirely?

What sort of expressions are allowed inside ${{ ... }}?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right two of the three values in the matrix are always the same. But the columns differ.
I see no way to omit the last column. Using null values there seems to make the workflow more complex as the default is used outside the matrix and would require to document it at least.

# Generated from:
# https://github.com/zopefoundation/meta/tree/master/config/pure-python
language: python
python:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe let's keep it for a bit longer, while we evaluate GitHub actions? It's not currently broken, is it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it in the PR so not each try to get it running runs on TravisCI using up our credits over there.

Copy link
Member Author

Choose a reason for hiding this comment

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

According to https://travis-ci.com/github/zopefoundation?tab=insights&timeInterval=week we used more than 1,000 minutes last week. So IMHO we should not eat up the Travis credits for repositories migrated to GHA.

Copy link
Member Author

@icemac icemac left a comment

Choose a reason for hiding this comment

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

I added a badge as well.

.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
- name: Test
run: tox -e {{ matrix.env || 'py'}}
run: tox -e ${{ matrix.config[2] }}
Copy link
Member Author

Choose a reason for hiding this comment

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

You are right two of the three values in the matrix are always the same. But the columns differ.
I see no way to omit the last column. Using null values there seems to make the workflow more complex as the default is used outside the matrix and would require to document it at least.

# Generated from:
# https://github.com/zopefoundation/meta/tree/master/config/pure-python
language: python
python:
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it in the PR so not each try to get it running runs on TravisCI using up our credits over there.

@icemac icemac merged commit 56581e9 into master Nov 17, 2020
@icemac icemac deleted the fix-gha branch November 17, 2020 07:11
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