Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: trunk-io/trunk-action
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v1.1.12
Choose a base ref
...
head repository: trunk-io/trunk-action
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v1.1.13
Choose a head ref
  • 1 commit
  • 3 files changed
  • 1 contributor

Commits on Apr 4, 2024

  1. feat: continue on node package install failure (#236) 

    ### Problem

Currently, most of our check-on-prs/check nightly failures occur while
installing node packages, for a variety of reasons. Right now, this
causes the check to fail, and we report that "something went wrong".
Instead, we may want to disable linters that depend on node-modules
(specifically, stylelint and eslint) and lint the remaining files to
report our best-effort results.

Pros:
- We would report some issues, and some issues is better than no issues
- We wouldn't be erroring with a message that seems like an internal
error (because for all we know right now, it could be)

Cons:
- A user could introduce a lint issue in the same PR as a breaking
change to package.json and we would pass that PR
- I am pretty sure that we don't have a great way of surfacing to the
user that we're disabling eslint without writing an annoying amount of
boilerplate to send the data through the cli through services back to
github
  - We do log this in the action logs, but that's the most visible place

### Solution

After talking with Sam about this, we figured the best solution is to
disable the linters if we are auto-initing for the user, but not if the
user has a trunk.yaml and therefore has deliberately opted to see ts/js
issues.

### Testing

Added a repo test for
[prawn-test-staging-rw/node-packages-failure-test](https://github.com/prawn-test-staging-rw/node-packages-failure-test/tree/main)
that has an invalid package.json, and checks to make sure the action
disables eslint and stylelint, and continues after the failure.
    @puzzler7
    puzzler7 authored Apr 4, 2024
    Loading
    Loading status checks…
    Copy the full SHA
    da67635 View commit details
Showing with 41 additions and 7 deletions.
  1. +20 −7 .github/workflows/repo_tests.yaml
  2. +1 −0 action.yaml
  3. +20 −0 setup-env/action.yaml
27 changes: 20 additions & 7 deletions .github/workflows/repo_tests.yaml
View file
Original file line number Diff line number Diff line change
@@ -35,7 +35,8 @@ jobs:
description: (compile-commands.json)
post-init: |
# black complains about py2 code
${TRUNK_PATH} check disable black
# markdownlint fails for some reason, and what we really care about anyways is clang-tidy
${TRUNK_PATH} check disable black markdownlint
mkdir build
cd build
cmake .. -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
@@ -44,8 +45,6 @@ jobs:
sed -i "s|lint:|lint:\n compile_commands: json|" .trunk/trunk.yaml
cp local-action/repo_tests/yaml_cpp.yaml .trunk/user.yaml
${TRUNK_PATH} check enable clang-tidy
# markdownlint fails for some reason, and what we really care about anyways is clang-tidy
${TRUNK_PATH} check disable markdownlint
- repo: pallets/flask
ref: 4bcd4be6b7d69521115ef695a379361732bcaea6
@@ -75,6 +74,23 @@ jobs:
fi
description: (test for setup-node)

- repo: prawn-test-staging-rw/node-packages-failure-test
ref: main
post-init: |
if [ "${FAILED_NODE_PACKAGE_INSTALL}" != "true" ]; then
echo "::error::Node package install didn't fail"
exit 1
fi
if grep -q "✔ eslint" <(${TRUNK_PATH} check list --color=false); then
echo "::error::eslint not disabled"
exit 1
fi
if grep -q "✔ stylelint" <(${TRUNK_PATH} check list --color=false); then
echo "::error::stylelint not disabled"
exit 1
fi
description: (test for continuing on node package install failure)

- repo: replayio/devtools
ref: 730a9f0ddaafefc2a1a293d6924ce3910cd156ac
description: (has trunk.yaml)
@@ -95,10 +111,7 @@ jobs:
description: (has trunk.yaml)
post-init: |
# all of these linters have failures
${TRUNK_PATH} check disable svgo
${TRUNK_PATH} check disable golangci-lint
${TRUNK_PATH} check disable prettier
${TRUNK_PATH} check disable oxipng
${TRUNK_PATH} check disable svgo golangci-lint prettier oxipng
- repo: shopify/draggable
ref: e6cf325a98c11b8aefbfb626b7a91b95d1c340c9
1 change: 1 addition & 0 deletions action.yaml
View file
Original file line number Diff line number Diff line change
@@ -223,6 +223,7 @@ runs:
run: |
if [ ! -e .trunk/trunk.yaml ]; then
${TRUNK_PATH:-trunk} init
echo "INITIALIZED_TRUNK=true" >>$GITHUB_ENV
fi
- name: Detect setup strategy
20 changes: 20 additions & 0 deletions setup-env/action.yaml
View file
Original file line number Diff line number Diff line change
@@ -115,6 +115,26 @@ runs:
hashFiles(env.HASH_GLOB) }}

- name: Install ${{ env.PACKAGE_MANAGER }} packages
id: install_packages
if: env.PACKAGE_MANAGER && (env.NODE_VERSION_FILE || env.RUN_INSTALL_NODE_PACKAGES)
shell: bash
run: ${{ env.INSTALL_CMD }}
continue-on-error: true

- name: Check for package install
if: env.PACKAGE_MANAGER && (env.NODE_VERSION_FILE || env.RUN_INSTALL_NODE_PACKAGES)
shell: bash
run: |
if [ ${{ steps.install_packages.outcome }} == "success" ]; then
exit 0
fi
echo "FAILED_NODE_PACKAGE_INSTALL=true" >>$GITHUB_ENV
if [[ -z "${INITIALIZED_TRUNK}" ]]; then
echo "::error::Failed to install node packages."
echo "::error::Aborting because this repo has an existing trunk.yaml file."
exit 1
fi
echo "::warning::Failed to install node packages."
echo "::warning::Disabling linters that depend on node packages."
${TRUNK_PATH} check disable eslint stylelint