-
Notifications
You must be signed in to change notification settings - Fork 561
chores: assorted fixes for release.yaml
#3134
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
Merged
+53
−59
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ jobs: | |
| runs-on: ubuntu-latest | ||
| name: Get release version from package.json | ||
| outputs: | ||
| package_version: ${{ steps.get_version.outputs.version }} | ||
| package_version: ${{ steps.get_version.outputs.package_version }} | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
@@ -32,8 +32,8 @@ jobs: | |
| - name: Validate inputs | ||
| env: | ||
| REF_NAME: ${{ github.ref_name }} | ||
| PACKAGE_NAME_INPUT: ${{ github.event.inputs.package_name }} | ||
| NPMJS_DIST_TAG_INPUT: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| PKG_NAME: ${{ github.event.inputs.package_name }} | ||
| NPM_DIST_TAG: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| run: | | ||
| set -euo pipefail | ||
| RELEASE_BRANCH="$(git branch --show-current || true)" | ||
|
|
@@ -47,7 +47,6 @@ jobs: | |
| fi | ||
|
|
||
| # Validate package_name | ||
| PKG_NAME="${PACKAGE_NAME_INPUT}" | ||
| if ! [[ "${PKG_NAME}" =~ ^[a-z0-9][a-z0-9-]*$ ]]; then | ||
| echo "❌ Invalid package_name '${PKG_NAME}' (allowed: [a-z0-9-], must start with alnum)." >&2 | ||
| exit 1 | ||
|
|
@@ -71,37 +70,35 @@ jobs: | |
| fi | ||
|
|
||
| # validate dist tag | ||
| NPM_DIST_TAG="${NPMJS_DIST_TAG_INPUT}" | ||
|
|
||
| # Empty → default to 'latest' | ||
| if [ -z "${NPM_DIST_TAG}" ]; then | ||
| NPM_DIST_TAG="latest" | ||
| DIST_TAG="${NPM_DIST_TAG}" | ||
| if [ -z "${DIST_TAG}" ]; then | ||
| DIST_TAG="latest" | ||
| echo "ℹ️ npmjs_dist_tag empty → defaulting to 'latest'." | ||
| fi | ||
|
|
||
| # Must start with a lowercase letter; then [a-z0-9._-]; max 128 chars | ||
| if ! [[ "${NPM_DIST_TAG}" =~ ^[a-z][a-z0-9._-]{0,127}$ ]]; then | ||
| echo "❌ Invalid npm dist-tag '${NPM_DIST_TAG}'. Must start with a lowercase letter and contain only [a-z0-9._-], max 128 chars." >&2 | ||
| if ! [[ "${DIST_TAG}" =~ ^[a-z][a-z0-9._-]{0,127}$ ]]; then | ||
| echo "❌ Invalid npm dist-tag '${DIST_TAG}'. Must start with a lowercase letter and contain only [a-z0-9._-], max 128 chars." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Disallow version-like prefixes (avoid semver/range confusion) | ||
| if [[ "${NPM_DIST_TAG}" =~ ^v[0-9] || "${NPM_DIST_TAG}" =~ ^[0-9] ]]; then | ||
| echo "❌ Invalid npm dist-tag '${NPM_DIST_TAG}'. Must not start with 'v' + digit or a digit (e.g., 'v1', '1.2.3')." >&2 | ||
| if [[ "${DIST_TAG}" =~ ^v[0-9] || "${DIST_TAG}" =~ ^[0-9] ]]; then | ||
| echo "❌ Invalid npm dist-tag '${DIST_TAG}'. Must not start with 'v' + digit or a digit (e.g., 'v1', '1.2.3')." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "✅ npmjs_dist_tag '${NPM_DIST_TAG}' is valid." | ||
| echo "✅ npmjs_dist_tag '${DIST_TAG}' is valid." | ||
|
|
||
| - name: Get package version from package.json | ||
| id: get_version | ||
| env: | ||
| PACKAGE_NAME_INPUT: ${{ github.event.inputs.package_name }} | ||
| NPMJS_DIST_TAG_INPUT: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| PKG_NAME: ${{ github.event.inputs.package_name }} | ||
| NPM_DIST_TAG: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| run: | | ||
| set -euo pipefail | ||
| PACKAGE_NAME="${PACKAGE_NAME_INPUT}" | ||
| PKG_JSON="packages/${PACKAGE_NAME}/package.json" | ||
| PKG_JSON="packages/${PKG_NAME}/package.json" | ||
| if [[ ! -f "${PKG_JSON}" ]]; then | ||
| echo "package.json not found at ${PKG_JSON}. Check 'package_name' input." >&2 | ||
| exit 1 | ||
|
|
@@ -111,16 +108,15 @@ jobs: | |
| echo "Version is empty or missing in ${PKG_JSON}" >&2 | ||
| exit 1 | ||
| fi | ||
| NPM_DIST_TAG="${NPMJS_DIST_TAG_INPUT}" | ||
| if [ -z "${NPM_DIST_TAG}" ]; then | ||
| NPM_DIST_TAG="latest" | ||
| DIST_TAG="${NPM_DIST_TAG}" | ||
| if [ -z "${DIST_TAG}" ]; then | ||
| DIST_TAG="latest" | ||
| fi | ||
| if [[ "${NPM_DIST_TAG}" == "latest" ]] && ! [[ "${VERSION}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | ||
| if [[ "${DIST_TAG}" == "latest" ]] && ! [[ "${VERSION}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | ||
| echo "With npmjs_dist_tag 'latest', version must be of the form x.y.z. Found '${VERSION}'." >&2 | ||
| exit 1 | ||
| fi | ||
| echo "PACKAGE_VERSION=${VERSION}" >> "$GITHUB_ENV" | ||
| echo "version=${VERSION}" >> "$GITHUB_OUTPUT" | ||
| echo "package_version=${VERSION}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| run_faucet_test: | ||
| name: Run faucet tests ${{ needs.get_version.outputs.package_version }} | ||
|
|
@@ -149,8 +145,8 @@ jobs: | |
| permissions: | ||
| issues: write | ||
| env: | ||
| PACKAGE_VERSION: "${{ needs.get_version.outputs.package_version }}" | ||
| PACKAGE_NAME: "${{ github.event.inputs.package_name }}" | ||
| PKG_VERSION: "${{ needs.get_version.outputs.package_version }}" | ||
| PKG_NAME: "${{ github.event.inputs.package_name }}" | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
@@ -183,7 +179,7 @@ jobs: | |
| REPO: ${{ github.repository }} | ||
| RUN_ID: ${{ github.run_id }} | ||
| run: | | ||
| MESSAGE="❌ Build failed for xrpl.js ${PACKAGE_VERSION}. Check the logs: https://github.com/${REPO}/actions/runs/${RUN_ID}" | ||
| MESSAGE="❌ Build failed for xrpl.js ${PKG_VERSION}. Check the logs: https://github.com/${REPO}/actions/runs/${RUN_ID}" | ||
| curl -X POST https://slack.com/api/chat.postMessage \ | ||
| -H "Authorization: Bearer ${SLACK_TOKEN}" \ | ||
| -H "Content-Type: application/json" \ | ||
|
|
@@ -216,7 +212,7 @@ jobs: | |
| -H "X-Api-Key: ${OWASP_TOKEN}" \ | ||
| -F "autoCreate=true" \ | ||
| -F "projectName=xrpl-js" \ | ||
| -F "projectVersion=${PACKAGE_VERSION}" \ | ||
| -F "projectVersion=${PKG_VERSION}" \ | ||
| -F "[email protected]" \ | ||
| https://owasp-dt-api.prod.ripplex.io/api/v1/bom | ||
|
|
||
|
|
@@ -269,13 +265,13 @@ jobs: | |
| LABELS: security | ||
| run: | | ||
| set -euo pipefail | ||
| TITLE="🔒 Security vulnerabilities in ${PACKAGE_NAME}@${PACKAGE_VERSION}" | ||
| TITLE="🔒 Security vulnerabilities in ${PKG_NAME}@${PKG_VERSION}" | ||
| : > issue_body.md | ||
|
|
||
| echo "The vulnerability scan has detected **CRITICAL/HIGH** vulnerabilities for \`${PACKAGE_NAME}@${PACKAGE_VERSION}\` on branch \`${REL_BRANCH}\`." >> issue_body.md | ||
| echo "The vulnerability scan has detected **CRITICAL/HIGH** vulnerabilities for \`${PKG_NAME}@${PKG_VERSION}\` on branch \`${REL_BRANCH}\`." >> issue_body.md | ||
| echo "" >> issue_body.md | ||
| echo "**Release Branch:** \`${REL_BRANCH}\`" >> issue_body.md | ||
| echo "**Package Version:** \`${PACKAGE_VERSION}\`" >> issue_body.md | ||
| echo "**Package Version:** \`${PKG_VERSION}\`" >> issue_body.md | ||
| echo "" >> issue_body.md | ||
| echo "**Full vulnerability report:** ${VULN_ART_URL}" >> issue_body.md | ||
| echo "" >> issue_body.md | ||
|
|
@@ -287,20 +283,20 @@ jobs: | |
|
|
||
| - name: Generate lerna.json for choosen the package | ||
| run: | | ||
| echo "🔧 Updating lerna.json to include only packages/${PACKAGE_NAME}" | ||
| echo "🔧 Updating lerna.json to include only packages/${PKG_NAME}" | ||
| # Use jq to update the packages field safely | ||
| jq --arg pkg "packages/${PACKAGE_NAME}" '.packages = [$pkg]' lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json | ||
| jq --arg pkg "packages/${PKG_NAME}" '.packages = [$pkg]' lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json | ||
| echo "✅ lerna.json updated:" | ||
| cat lerna.json | ||
|
|
||
| - name: Pack tarball | ||
| run: | | ||
| set -euo pipefail | ||
| echo "Packaging ${PACKAGE_NAME}" | ||
| find "packages/${PACKAGE_NAME}" -maxdepth 1 -name '*.tgz' -delete || true | ||
| FULL_PACKAGE_NAME="$(jq -er '.name' packages/${PACKAGE_NAME}/package.json)" | ||
| TARBALL=$(npx lerna exec --scope "${FULL_PACKAGE_NAME}" -- npm pack --json | jq -r '.[0].filename') | ||
| echo "TARBALL=packages/${PACKAGE_NAME}/${TARBALL}" >> "$GITHUB_ENV" | ||
| echo "Packaging ${PKG_NAME}" | ||
| find "packages/${PKG_NAME}" -maxdepth 1 -name '*.tgz' -delete || true | ||
| FULL_PKG_NAME="$(jq -er '.name' packages/${PKG_NAME}/package.json)" | ||
| TARBALL=$(npx lerna exec --scope "${FULL_PKG_NAME}" -- npm pack --json | jq -r '.[0].filename') | ||
| echo "TARBALL=packages/${PKG_NAME}/${TARBALL}" >> "$GITHUB_ENV" | ||
|
|
||
| - name: Upload tarball as artifact | ||
| uses: actions/upload-artifact@v4 | ||
|
|
@@ -315,8 +311,8 @@ jobs: | |
| pull-requests: write | ||
| name: Print Test/Security scan result and invite Dev team to review | ||
| env: | ||
| PACKAGE_VERSION: "${{ needs.get_version.outputs.package_version }}" | ||
| PACKAGE_NAME: "${{ github.event.inputs.package_name }}" | ||
| PKG_VERSION: "${{ needs.get_version.outputs.package_version }}" | ||
| PKG_NAME: "${{ github.event.inputs.package_name }}" | ||
| RELEASE_BRANCH: "${{ github.ref_name }}" | ||
| outputs: | ||
| reviewers_dev: ${{ steps.get_reviewers.outputs.reviewers_dev }} | ||
|
|
@@ -380,7 +376,6 @@ jobs: | |
| RUN_ID: ${{ github.run_id }} | ||
| ENV_DEV_NAME: first-review | ||
| ENV_SEC_NAME: official-release | ||
| NPMJS_DIST_TAG: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| PR_URL: ${{ steps.ensure_pr.outputs.pr_url }} | ||
| GITHUB_ACTOR: ${{ github.actor }} | ||
| GITHUB_TRIGGERING_ACTOR: ${{ github.triggering_actor }} | ||
|
|
@@ -429,7 +424,6 @@ jobs: | |
| REPO: ${{ github.repository }} | ||
| RUN_ID: ${{ github.run_id }} | ||
| ENV_NAME: official-release | ||
| NPMJS_DIST_TAG: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| GITHUB_ACTOR: ${{ github.actor }} | ||
| GITHUB_TRIGGERING_ACTOR: ${{ github.triggering_actor }} | ||
| PR_URL: ${{ steps.ensure_pr.outputs.pr_url }} | ||
|
|
@@ -452,8 +446,8 @@ jobs: | |
| fi | ||
|
|
||
| echo "🔍 Please review the following details before proceeding:" | ||
| echo "📦 Package Name: ${PACKAGE_NAME}" | ||
| echo "🔖 Package Version: ${PACKAGE_VERSION}" | ||
| echo "📦 Package Name: ${PKG_NAME}" | ||
| echo "🔖 Package Version: ${PKG_VERSION}" | ||
| echo "🌿 Release Branch: ${RELEASE_BRANCH}" | ||
| echo "🔢 Commit SHA: ${COMMIT_SHA}" | ||
| echo "🔗 Vulnerabilities: https://github.com/${REPO}/actions/runs/${RUN_ID}/artifacts/${ARTIFACT_ID}" | ||
|
|
@@ -473,7 +467,7 @@ jobs: | |
| set -euo pipefail | ||
| RUN_URL="https://github.com/${REPO}/actions/runs/${RUN_ID}" | ||
|
|
||
| MSG="${EXECUTOR} is releasing ${PACKAGE_NAME}@${PACKAGE_VERSION}. A member from the dev team (${DEV_REVIEWERS}) needs to take the following actions: \n1) Review the release artifacts and approve/reject the release. (${RUN_URL})" | ||
| MSG="${EXECUTOR} is releasing ${PKG_NAME}@${PKG_VERSION}. A member from the dev team (${DEV_REVIEWERS}) needs to take the following actions: \n1) Review the release artifacts and approve/reject the release. (${RUN_URL})" | ||
|
|
||
| if [ -n "${PR_URL}" ]; then | ||
| MSG="${MSG} \n2) Review the package update PR and provide two approvals. DO NOT MERGE — ${EXECUTOR} will verify the package on npm and merge the approved PR. (${PR_URL})" | ||
|
|
@@ -523,16 +517,16 @@ jobs: | |
| SLACK_TOKEN: ${{ secrets.SLACK_TOKEN }} | ||
| CHANNEL: "#ripplex-security" | ||
| EXECUTOR: ${{ github.triggering_actor || github.actor }} | ||
| PACKAGE_NAME: ${{ needs.get_version.outputs.package_version && github.event.inputs.package_name }} | ||
| PACKAGE_VERSION: ${{ needs.get_version.outputs.package_version }} | ||
| PKG_NAME: ${{ github.event.inputs.package_name }} | ||
| PKG_VERSION: ${{ needs.get_version.outputs.package_version }} | ||
| REPO: ${{ github.repository }} | ||
| RUN_ID: ${{ github.run_id }} | ||
| SEC_REVIEWERS: ${{ needs.ask_for_dev_team_review.outputs.reviewers_sec }} | ||
| run: | | ||
| set -euo pipefail | ||
| RUN_URL="https://github.com/${REPO}/actions/runs/${RUN_ID}" | ||
|
|
||
| MSG="${EXECUTOR} is releasing ${PACKAGE_NAME}@${PACKAGE_VERSION}. A member from the infosec team (${SEC_REVIEWERS}) needs to take the following action:\n Review the release artifacts and approve/reject the release. (${RUN_URL})" | ||
| MSG="${EXECUTOR} is releasing ${PKG_NAME}@${PKG_VERSION}. A member from the infosec team (${SEC_REVIEWERS}) needs to take the following action:\n Review the release artifacts and approve/reject the release. (${RUN_URL})" | ||
| MSG=$(printf '%b' "${MSG}") | ||
| curl -sS -X POST https://slack.com/api/chat.postMessage \ | ||
| -H "Authorization: Bearer ${SLACK_TOKEN}" \ | ||
|
|
@@ -557,8 +551,8 @@ jobs: | |
| ] | ||
| name: Release for ${{ needs.get_version.outputs.package_version }} | ||
| env: | ||
| PACKAGE_VERSION: "${{ needs.get_version.outputs.package_version }}" | ||
| PACKAGE_NAME: "${{ github.event.inputs.package_name }}" | ||
| PKG_VERSION: "${{ needs.get_version.outputs.package_version }}" | ||
| PKG_NAME: "${{ github.event.inputs.package_name }}" | ||
| environment: | ||
| name: official-release | ||
| url: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }} | ||
|
|
@@ -588,27 +582,27 @@ jobs: | |
|
|
||
| - name: Publish to npm | ||
| env: | ||
| NPMJS_DIST_TAG_INPUT: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| NPM_DIST_TAG: ${{ github.event.inputs.npmjs_dist_tag }} | ||
| run: | | ||
| cd dist | ||
| PKG=$(ls *.tgz) | ||
| echo ${PKG} | ||
| NPM_DIST_TAG="${NPMJS_DIST_TAG_INPUT}" | ||
| if [ -z "${NPM_DIST_TAG}" ]; then | ||
| NPM_DIST_TAG="latest" | ||
| DIST_TAG="${NPM_DIST_TAG}" | ||
| if [ -z "${DIST_TAG}" ]; then | ||
| DIST_TAG="latest" | ||
| fi | ||
| if [[ "${NPM_DIST_TAG}" == "latest" ]] && ! [[ "${PACKAGE_VERSION}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | ||
| echo "With npmjs_dist_tag 'latest', version must be of the form x.y.z. Found '${PACKAGE_VERSION}'." >&2 | ||
| if [[ "${DIST_TAG}" == "latest" ]] && ! [[ "${PKG_VERSION}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | ||
| echo "With npmjs_dist_tag 'latest', version must be of the form x.y.z. Found '${PKG_VERSION}'." >&2 | ||
| exit 1 | ||
| fi | ||
| npm i -g [email protected] | ||
| npm publish "${PKG}" --provenance --access public --registry=https://registry.npmjs.org/ --tag "${NPM_DIST_TAG}" | ||
| npm publish "${PKG}" --provenance --access public --registry=https://registry.npmjs.org/ --tag "${DIST_TAG}" | ||
|
|
||
| - name: Ensure Git tag exists | ||
| id: create_tag | ||
| run: | | ||
| set -euo pipefail | ||
| TAG="${PACKAGE_NAME}@${PACKAGE_VERSION}" | ||
| TAG="${PKG_NAME}@${PKG_VERSION}" | ||
|
|
||
| git fetch --tags origin | ||
|
|
||
|
|
@@ -646,7 +640,7 @@ jobs: | |
| enc_tag="$(printf '%s' "${TAG}" | jq -sRr @uri)" | ||
| RELEASE_URL="https://github.com/${REPO}/releases/tag/${enc_tag}" | ||
|
|
||
| text="${PACKAGE_NAME} ${PACKAGE_VERSION} has been succesfully released and published to npm.js. Release URL: ${RELEASE_URL}" | ||
| text="${PKG_NAME} ${PKG_VERSION} has been succesfully released and published to npm.js. Release URL: ${RELEASE_URL}" | ||
| text="${text//\\n/ }" | ||
|
|
||
| curl -sS -X POST https://slack.com/api/chat.postMessage \ | ||
|
|
@@ -661,7 +655,7 @@ jobs: | |
| REPO: ${{ github.repository }} | ||
| RUN_ID: ${{ github.run_id }} | ||
| run: | | ||
| MESSAGE="❌ Release failed for ${PACKAGE_NAME}@${PACKAGE_VERSION}. Check the logs: https://github.com/${REPO}/actions/runs/${RUN_ID}" | ||
| MESSAGE="❌ Release failed for ${PKG_NAME}@${PKG_VERSION}. Check the logs: https://github.com/${REPO}/actions/runs/${RUN_ID}" | ||
| curl -X POST https://slack.com/api/chat.postMessage \ | ||
| -H "Authorization: Bearer ${SLACK_TOKEN}" \ | ||
| -H "Content-Type: application/json" \ | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
You can just name this
DIST_TAGdirectly, since in the script below you callDIST_TAG="${NPM_DIST_TAG}"and don't useNPM_DIST_TAGfor anything else.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.
In line 76 we reassign the value to
latestwhich I believe is not allowed for env variable, that's why we need an intermediate value. However, this is only used for printing out messages so we can use something like${NPM_DIST_TAG:-latest}. Let me know what you thinkThere 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.
Are you saying that if
${{ github.event.inputs.npmjs_dist_tag }}is "latest" then GitHub will not execute the workflow?Assuming you'd rename
NPM_DIST_TAGtoDIST_TAGhere in theenv:section, then below you can just do:no? I've never heard of the value "latest" not being allowed as an env var, that would be very odd. (then again, Gmail doesn't allow you to use a label "Finance", so sometimes things one sees in the world are just strange).
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.
Actually, you are right, it's just a good practice not to reassign env variable since it is shared within the job, but you can certainly do so. Also, in our case it does not affect anything since these steps are at the end of the job. However, since this is pretty minor, I would prefer to keep it as is just to follow good practice
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.
That's fair. I don't know if next steps will use the original or the modified env var.
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.
btw you can also just do:
and then you don't need the check in the script.