-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update: [AEA-3806] - add linting to GitHub actions #135
Conversation
slight change to asdf cache because it doesn't update if the packages change
removed name from pyproject.toml to stop duplicate directories being created when creating the virtual env set path toe virtual env for the cache to what the documentation says it should look like
@@ -26,8 +26,6 @@ jobs: | |||
tag_release: | |||
needs: quality_checks | |||
runs-on: ubuntu-latest | |||
outputs: |
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.
Does this want output_version_tag like in release.yml?
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.
added
continue-on-error: true | ||
|
||
- name: Grab ticket name | ||
if: contains(github.event.pull_request.head.ref, 'aea-') || contains(github.event.pull_request.head.ref, 'AEA-') || contains(github.event.pull_request.head.ref, 'apm-') || contains(github.event.pull_request.head.ref, 'APM-') || contains(github.event.pull_request.head.ref, 'apmspii-') || contains(github.event.pull_request.head.ref, 'APMSPII-') || contains(github.event.pull_request.head.ref, 'adz-') || contains(github.event.pull_request.head.ref, 'ADZ-') || contains(github.event.pull_request.head.ref, 'amb-') || contains(github.event.pull_request.head.ref, 'AMB-') | ||
run: echo ::set-env name=TICKET_NAME::$(echo ${{ github.event.pull_request.head.ref }} | grep -i -o '\(aea-[0-9]\+\)\|\(apm-[0-9]\+\)\|\(apmspii-[0-9]\+\)\|\(adz-[0-9]\+\)|\(amb-[0-9]\+\)' | tr '[:lower:]' '[:upper:]') | ||
if: contains(github.event.pull_request.head.ref, 'aea-') || contains(github.event.pull_request.head.ref, 'AEA-') |
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.
Should $REF also be used in here?
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.
There doesn't seem to be a way to do this inside of the contains() method. I checked the EPS API and it's done the same way as this currently
continue-on-error: true | ||
env: | ||
ACTIONS_ALLOW_UNSECURE_COMMANDS: true | ||
|
||
- name: Comment on PR with link to JIRA ticket | ||
if: contains(github.event.pull_request.head.ref, 'aea-') || contains(github.event.pull_request.head.ref, 'AEA-') || contains(github.event.pull_request.head.ref, 'apm-') || contains(github.event.pull_request.head.ref, 'APM-') || contains(github.event.pull_request.head.ref, 'apmspii-') || contains(github.event.pull_request.head.ref, 'APMSPII-') || contains(github.event.pull_request.head.ref, 'adz-') || contains(github.event.pull_request.head.ref, 'ADZ-') || contains(github.event.pull_request.head.ref, 'amb-') || contains(github.event.pull_request.head.ref, 'AMB-') | ||
if: contains(github.event.pull_request.head.ref, 'aea-') || contains(github.event.pull_request.head.ref, 'AEA-') |
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.
Should $REF also also be used in here?
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.
There doesn't seem to be a way to do this inside of the contains() method. I checked the EPS API and it's done the same way as this currently
path: ./.venv/ | ||
key: ${{ runner.os }}-${{ steps.setup_python.outputs.python-version }}-venv-${{ hashFiles('pyproject.toml') }} | ||
path: .venv | ||
key: ${{ runner.os }}-venv-${{ hashFiles('pyproject.toml') }} |
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.
Have these changes been tested?
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.
This doesn't work either way. I have a tech-debt ticket to address this: https://nhsd-jira.digital.nhs.uk/browse/AEA-4199
|
Summary
Remove items from this list if they are not relevant. Remove this line once this has been done
Details
actionlint added to pre-commit to ensure linting is enforced on github actions .yml files