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

refactor: lint shell for shellcheck #2193

Merged
merged 1 commit into from
Nov 4, 2021
Merged

refactor: lint shell for shellcheck #2193

merged 1 commit into from
Nov 4, 2021

Conversation

zaunist
Copy link
Contributor

@zaunist zaunist commented Nov 1, 2021

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

Refer to shellcheck to standardize shell scripting.
example:

pwd=`pwd`

to:

pwd=$(pwd)

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2021

Codecov Report

Merging #2193 (d936c95) into master (e1e83e3) will decrease coverage by 2.33%.
The diff coverage is n/a.

❗ Current head d936c95 differs from pull request most recent head e7c45b6. Consider uploading reports for the commit e7c45b6 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2193      +/-   ##
==========================================
- Coverage   69.45%   67.11%   -2.34%     
==========================================
  Files         189       62     -127     
  Lines        7281     3929    -3352     
  Branches      824        0     -824     
==========================================
- Hits         5057     2637    -2420     
+ Misses       1927      997     -930     
+ Partials      297      295       -2     
Flag Coverage Δ
backend-e2e-test ?
backend-e2e-test-ginkgo 49.78% <ø> (+0.12%) ⬆️
backend-unit-test 50.39% <ø> (+0.03%) ⬆️
frontend-e2e-test ?

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

Impacted Files Coverage Δ
api/internal/handler/data_loader/route_import.go 32.11% <0.00%> (-35.41%) ⬇️
api/internal/handler/global_rule/global_rule.go 68.11% <0.00%> (-17.40%) ⬇️
api/internal/core/entity/entity.go 77.27% <0.00%> (-13.64%) ⬇️
api/internal/utils/utils.go 60.00% <0.00%> (-13.00%) ⬇️
api/internal/core/store/storehub.go 68.80% <0.00%> (-3.21%) ⬇️
api/internal/core/store/validate.go 68.68% <0.00%> (-2.03%) ⬇️
api/internal/handler/label/label.go 84.48% <0.00%> (-1.73%) ⬇️
api/internal/core/store/store.go 88.54% <0.00%> (-0.53%) ⬇️
...nents/Upstream/components/ServiceDiscoveryArgs.tsx
.../components/passive-check/Healthy/HttpStatuses.tsx
... and 125 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1e83e3...e7c45b6. Read the comment docs.

@zaunist
Copy link
Contributor Author

zaunist commented Nov 1, 2021

@bzp2010
Copy link
Contributor

bzp2010 commented Nov 1, 2021

@zaunist I think we can leave the changes to cli_test.sh here, it will cause a conflict with another PR [1] that actually refactors it.

[1] #2183

@zaunist
Copy link
Contributor Author

zaunist commented Nov 1, 2021

@zaunist I think we can leave the changes to cli_test.sh here, it will cause a conflict with another PR [1] that actually refactors it.

[1] #2183

Yes, I have undone the changes to cli_test.sh

@zaunist
Copy link
Contributor Author

zaunist commented Nov 4, 2021

cc @bzp2010

@nic-chen nic-chen merged commit 5510ecb into apache:master Nov 4, 2021
@zaunist zaunist deleted the shell_check branch November 4, 2021 07:42
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.

8 participants