-
-
Notifications
You must be signed in to change notification settings - Fork 751
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
Add GHA workflow to lint with pantsbuild #5758
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,88 @@ | ||||
--- | ||||
# This Lint workflow uses pants | ||||
name: Lint | ||||
|
||||
on: | ||||
push: | ||||
branches: | ||||
# only on merges to master branch | ||||
- master | ||||
# and version branches, which only include minor versions (eg: v3.4) | ||||
- v[0-9]+.[0-9]+ | ||||
tags: | ||||
# also version tags, which include bugfix releases (eg: v3.4.0) | ||||
- v[0-9]+.[0-9]+.[0-9]+ | ||||
pull_request: | ||||
type: [opened, reopened, edited] | ||||
branches: | ||||
# Only for PRs targeting those branches | ||||
- master | ||||
- v[0-9]+.[0-9]+ | ||||
schedule: | ||||
# run every night at midnight | ||||
- cron: '0 0 * * *' | ||||
|
||||
jobs: | ||||
# Lint checks which don't depend on any service containes, etc. to be running. | ||||
lint-checks: | ||||
name: 'Lint Checks (pants runs: shellcheck)' | ||||
runs-on: ubuntu-latest | ||||
|
||||
env: | ||||
COLUMNS: '120' | ||||
|
||||
steps: | ||||
- name: Checkout repository | ||||
uses: actions/checkout@v2 | ||||
with: | ||||
# a test uses a submodule, and pants needs access to it to calculate deps. | ||||
submodules: 'true' | ||||
|
||||
#- name: Cache APT Dependencies | ||||
# id: cache-apt-deps | ||||
# uses: actions/cache@v2 | ||||
# with: | ||||
# path: | | ||||
# ~/apt_cache | ||||
# key: ${{ runner.os }}-apt-v7-${{ hashFiles('scripts/github/apt-packages.txt') }} | ||||
# restore-keys: | | ||||
# ${{ runner.os }}-apt-v7- | ||||
- name: Install APT Depedencies | ||||
env: | ||||
CACHE_HIT: 'false' # cache doesn't work | ||||
#CACHE_HIT: ${{steps.cache-apt-deps.outputs.cache-hit}} | ||||
run: | | ||||
# install dev dependencies for Python YAML and LDAP packages | ||||
# https://github.com/StackStorm/st2-auth-ldap | ||||
./scripts/github/install-apt-packages-use-cache.sh | ||||
|
||||
- name: Initialize Pants and its GHA caches | ||||
uses: pantsbuild/actions/init-pants@c0ce05ee4ba288bb2a729a2b77294e9cb6ab66f7 | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't we point to master or latest release or something here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This repo does not yet have tags we can point to. If there are changes, we might need to adjust how we use the action, app pinning to a particular version seemed the best way to go. When we choose another approach (eg when there are tags available), then we'll need to change the pants BUILD validation workflow add well: st2/.github/workflows/pants.yaml Line 33 in 989e4ec
|
||||
# This action adds an env var to make pants use both pants.ci.toml & pants.toml. | ||||
# This action also creates 3 GHA caches (1 is optional). | ||||
# - `pants-setup` has the bootsrapped pants install | ||||
# - `pants-named-caches` has pip/wheel and PEX caches | ||||
# - `pants-lmdb-store` has the fine-grained process cache. | ||||
# If we ever use a remote cache, then we can drop this. | ||||
# Otherwise, we may need an additional workflow or job to delete old caches | ||||
# if they are not expiring fast enough, and we hit the GHA 10GB per repo max. | ||||
with: | ||||
base-branch: master | ||||
# To ignore a bad cache, bump the cache* integer. | ||||
gha-cache-key: cache0 | ||||
# This hash should include all of our lockfiles so that the pip/pex caches | ||||
# get invalidated on any transitive dependency update. | ||||
named-caches-hash: ${{ hashFiles('requirements.txt') }} | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the root level There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not entirely. I plan to change this to use the lockfiles instead once the lockfiles are available. |
||||
# enable the optional lmdb_store cache since we're not using remote caching. | ||||
cache-lmdb-store: 'true' | ||||
|
||||
- name: Lint | ||||
run: | | ||||
./pants lint :: | ||||
|
||||
- name: Upload pants log | ||||
uses: actions/upload-artifact@v2 | ||||
with: | ||||
name: pants-log-py${{ matrix.python-version }} | ||||
path: .pants.d/pants.log | ||||
if: always() # We want the log even on failures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied that from our other workflows which we run every day. We did that in part to catch issues with transitive deps, which will not be a significant issue once we've got the lockfiles in. So, I suppose it is unlikely for linters to fail from one day to another, so maybe we don't have to be quite so aggressive here. 🤔
What do others think? Drop this? Comment it out for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that you already are using "tool lockfiles". Pants ships them by default. So your installation of tools like Flake8 and Pylint will be deterministic. It is only installs of your own user code that need to manually set up lockfiles, since we can't ship them without knowing before what your code is.
So running this is cron is likely a waste of compute resources (and trees)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commented it out.