Skip to content

Conversation

@AaronZyLee
Copy link
Contributor

@AaronZyLee AaronZyLee commented Jul 12, 2023

Description of changes

Fix the issue of lint error not emitting

CDK / CloudFormation Parameters Changed

Issue #, if available

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies
  • Any CDK or CloudFormation parameter changes are called out explicitly

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@AaronZyLee AaronZyLee requested a review from a team as a code owner July 12, 2023 21:21

# get PR file list, filter out removed files, filter only JS/TS files, then pass to the linter
curl -fsSL https://api.github.com/repos/$PROJECT_USERNAME/$REPO_NAME/pulls/$PR_NUM/files | jq -r '.[] | select(.status!="removed") | .filename' | grep -E '\.(js|jsx|ts|tsx)$' || true | xargs yarn eslint
curl -fsSL https://api.github.com/repos/$PROJECT_USERNAME/$REPO_NAME/pulls/$PR_NUM/files | jq -r '.[] | select(.status!="removed") | .filename' | (grep -E '\.(js|jsx|ts|tsx)$' || true) | xargs yarn eslint
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parenthesis is required here. Otherwise it won't emit the lint error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the same bug in circle-ci lint script as well, but at this point I don't think we care much about fixing that.

phani-srikar
phani-srikar previously approved these changes Jul 12, 2023
dpilch
dpilch previously approved these changes Jul 13, 2023
@dpilch dpilch dismissed stale reviews from phani-srikar and themself via 8422e85 July 2, 2024 16:33
@dpilch
Copy link
Contributor

dpilch commented Jul 2, 2024

I've tested this with a couple of open PRs.

  1. Has JS/TS files to lint: curl -fsSL https://api.github.com/repos/aws-amplify/amplify-category-api/pulls/2666/files | jq -r '.[] | select(.status!="removed") | .filename' | grep -E '\.(js|jsx|ts|tsx)$' | xargs yarn eslint || true
  2. Does not have JS/TS files to lint: curl -fsSL https://api.github.com/repos/aws-amplify/amplify-category-api/pulls/1665/files | jq -r '.[] | select(.status!="removed") | .filename' | grep -E '\.(js|jsx|ts|tsx)$' | xargs yarn eslint || true

@dpilch dpilch enabled auto-merge (squash) July 3, 2024 14:33
@dpilch dpilch merged commit b9d2864 into main Aug 7, 2024
@dpilch dpilch deleted the fix-lint branch August 7, 2024 17:57
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.

5 participants