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

chore: add QA checks to Test workflow #219

Merged
merged 5 commits into from
Apr 5, 2023
Merged

chore: add QA checks to Test workflow #219

merged 5 commits into from
Apr 5, 2023

Conversation

maxrake
Copy link
Contributor

@maxrake maxrake commented Apr 4, 2023

This change adds a new QA job to the existing Test workflow. It is done in a way that the Test rollup job can continue to be an enforced status check, but with the addition of this job. A new optional tox test environment named qa was added and is called by the QA job. The environment can also be called by users locally. It relies on simply running pre-commit on the existing hooks defined in the repository:

  • trim trailing whitespace
  • fix end of files
  • check yaml
  • check for added large files
  • black
  • pyupgrade
  • analyze lockfile with phylum-ci
    • This one is skipped

The phylum-ci pre-commit hook is skipped in the QA job since:

  • The current GitHub integration expects to only run in a PR context
    • The Test workflow also includes workflow_dispatch and push
    • The integration will fail even if a GITHUB_TOKEN is provided
  • The phylum-ci action will already be run for pull request triggers

It is possible this restriction can be lifted in the future if the
GitHub integration is updated to account for additional execution
contexts beyond pull requests.

The pre-commit package was added as a dependency in the qa group and can therefore be kept under configuration control and used by poetry. Documentation was updated to make all the new usage patterns explicit.

Additional QA checks and pre-commit hooks will be added separately.

Additional changes made include:

  • Fix a missing version check normalization
    • When no CLI version is specified and no CLI is installed
    • The version was reported as latest
    • The version is now the current latest version in full "vMajor.Minor.Bugfix" format

Closes #14

Checklist

  • Does this PR have an associated issue (i.e., closes #<issueNum> in description above)?
  • Have you ensured that you have met the expected acceptance criteria?
  • Have you created sufficient tests?
  • Have you updated all affected documentation?

This change adds a new `QA` job to the existing `Test` workflow. It is
done in a way that the `Test rollup` job can continue to be an enforced
status check, but with the addition of this job. A new optional `tox`
test environment named `qa` was added and is called by the `QA` job.
The environment can also be called by users locally. It relies on simply
running `pre-commit` on the existing hooks defined in the repository:

* trim trailing whitespace
* fix end of files
* check yaml
* check for added large files
* `black`
* `pyupgrade`
* analyze lockfile with `phylum-ci`

The `pre-commit` package was added as a dependency in the `qa` group and
can therefore be kept under configuration control and used by `poetry`.
Documentation was updated to make all the new usage patterns explicit.

Additional QA checks and `pre-commit` hooks will be added separatetely.
@maxrake maxrake requested a review from a team as a code owner April 4, 2023 18:02
@maxrake maxrake requested a review from andreaphylum April 4, 2023 18:02
@github-actions
Copy link

github-actions bot commented Apr 4, 2023

Phylum OSS Supply Chain Risk Analysis - SUCCESS

The Phylum risk analysis is complete and did not identify any issues.

View this project in the Phylum UI

kylewillmon
kylewillmon previously approved these changes Apr 4, 2023
maxrake added 2 commits April 4, 2023 13:22
When no CLI version is specified and no CLI is installed, the version is
reported as `latest` instead of the current latest version in full
"vMajor.Minor.Bugfix" format.
We need to skip the `phylum-ci` pre-commit hook since:

* The current GitHub integration expects to *only* run in a PR context
  * The `Test` workflow also includes `workflow_dispatch` and `push`
  * The integration will fail even if a `GITHUB_TOKEN` is provided
* The `phylum-ci` action will already be run for pull request triggers

It is possible this restriction can be lifted in the future if the
GitHub integration is updated to account for additional execution
contexts beyond pull requests.
@maxrake maxrake requested a review from kylewillmon April 4, 2023 19:03
@maxrake maxrake self-assigned this Apr 4, 2023
@maxrake maxrake enabled auto-merge (squash) April 4, 2023 20:05
@maxrake maxrake merged commit 6545116 into main Apr 5, 2023
@maxrake maxrake deleted the enforce_qa branch April 5, 2023 14:28
@maxrake
Copy link
Contributor Author

maxrake commented Apr 5, 2023

FYI...I tried to enable auto-merge on this PR and accidentally merged it outright instead. @kylewillmon took a look at the PR again (since his last review and after this accidental merge) and confirmed that it looked good to him.

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.

Add QA checks to workflow
2 participants