Skip to content
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

Change shellcheck severity level to warning #5996

Merged
merged 9 commits into from
Jan 8, 2024
Merged

Conversation

judysng
Copy link
Contributor

@judysng judysng commented Jan 5, 2024

Description of changes

  • Change the shellcheck severity level to warning to discover potential problems earlier
  • Ignored the tests/ folder (many warnings are from the tests)
  • Fixed all the other warnings, the warnings are in this check in my fork

Tests

  • Some edited files included batch resources, made a batch cluster
  • Ran integration test for pcluster API, they fail but with the same failures as develop, but ran gradlew on local and they all worked

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

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

@judysng judysng requested review from a team as code owners January 5, 2024 19:32
@judysng judysng added the skip-changelog-update Disables the check that enforces changelog updates in PRs label Jan 5, 2024
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1f441f1) 90.18% compared to head (9b3b709) 90.18%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #5996   +/-   ##
========================================
  Coverage    90.18%   90.18%           
========================================
  Files          180      180           
  Lines        15780    15780           
========================================
  Hits         14231    14231           
  Misses        1549     1549           
Flag Coverage Δ
unittests 90.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

api/gradlew Outdated
@@ -36,9 +36,9 @@ while [ -h "$PRG" ] ; do
fi
done
SAVED="`pwd`"
cd "`dirname \"$PRG\"`/" >/dev/null
cd "`dirname \"$PRG\"`/" >/dev/null || exit
Copy link
Contributor

Choose a reason for hiding this comment

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

we can use $() syntax as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated all the usages of ` in gradlew to use $()

Copy link
Contributor

@hanwen-pcluste hanwen-pcluste left a comment

Choose a reason for hiding this comment

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

Great work!

@judysng judysng merged commit a1dc3af into aws:develop Jan 8, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog-update Disables the check that enforces changelog updates in PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants