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

Added GitHub Actions Workflow #101

Closed
wants to merge 58 commits into from

Conversation

daminijain23
Copy link
Contributor

Added .github/workflows/tox-test.yml and .github/workflows/release.yml.
Removed .travis.yml

GitHub Actions and Travis workflows can run parallel. In such case, if both run successfully only then GitHub reports a success.
On reducing the number of tests, coverage also reduced.

.github/workflows/tox-test.yml Outdated Show resolved Hide resolved
- name: Install Tox
run: pip install tox
- name: Install pytest cov
run: pip install pytest-cov
Copy link
Member

Choose a reason for hiding this comment

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

I see, this makes sense, but may I suggest the following then:

  • this shouldn't be pytest-cov specifically because codecov-action doesn't depend on "pytest-cov", it only depends on "coverage" (i.e. not the pytest stuff) - right?
  • could we please move this to after we run "tox -e cov", i.e. just before we run codecov-action? I think that makes the intent clearer, that we're installing this just for that action. A brief comment mentioning that would also be nice.

.github/workflows/tox-test.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@rohanpm rohanpm closed this in 36082ba Aug 29, 2021
@rohanpm
Copy link
Member

rohanpm commented Aug 29, 2021

@daminijain23, I submitted most of the changes here, with commits squashed. Let's use a new PR if there are further changes needed.

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.

None yet

2 participants