Skip to content

Remove redundant tests#57

Open
s-weigand wants to merge 1 commit intoeconchick:masterfrom
s-weigand:remove-redundent-tests
Open

Remove redundant tests#57
s-weigand wants to merge 1 commit intoeconchick:masterfrom
s-weigand:remove-redundent-tests

Conversation

@s-weigand
Copy link
Contributor

Description

Since black is in your .pre-commit-config.yaml, having extra formatting tests is redundant + you would have to manage its version in two more places.
As for running pre-commit run -a locally, this will be done on staged files by the installed hook.
On the CIs 'Pre-commit' job all files will be tested and since it utilizes caching the tests should overall finish faster.

Motivation and Context

Failing CI tests du to check with an older version of black see #54

@s-weigand
Copy link
Contributor Author

Removing the CI restriction on push, to only run on master, would allow contributors to see the CI results on their forks feature branch before submitting a PR.

branches: [ master ]

branches: [ master ]

@s-weigand s-weigand force-pushed the remove-redundent-tests branch from 6a7d48a to c3d911b Compare October 6, 2020 23:36
Since `black` is in your `.pre-commit-config.yaml`, having extra formatting tests is redundant + you would have to manage its version in two more places.
As for running `pre-commit run -a` on the CI this is done by the 'Pre-commit' job, which utilizes caching.
@s-weigand s-weigand force-pushed the remove-redundent-tests branch from c3d911b to 52f8ce1 Compare October 14, 2020 08:40
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.

2 participants