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

move shfmt execution from the super linter to pre-commit #17913

Merged
merged 2 commits into from Apr 29, 2024

Conversation

altendky
Copy link
Contributor

@altendky altendky commented Apr 24, 2024

Purpose:

I have no idea what shell formatter we should use. But, #17906 was updating the super linter and it started complaining a lot and I really dislike having formatting tools executed in the super linter since that doesn't integrate into our local checks and thus people have to manually apply the super linter results to their local copy. So, this moves the super linter choice of shfmt into our pre-commit setup which runs both locally and in CI.

Current Behavior:

New Behavior:

Testing Notes:

@altendky altendky requested a review from a team as a code owner April 24, 2024 15:34
@altendky altendky added Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes labels Apr 24, 2024
Copy link

coveralls-official bot commented Apr 24, 2024

Pull Request Test Coverage Report for Build 8832083974

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 474 unchanged lines in 28 files lost coverage.
  • Overall coverage decreased (-0.4%) to 90.651%

Files with Coverage Reduction New Missed Lines %
chia/util/config.py 1 84.64%
chia/util/service_groups.py 1 83.33%
chia/rpc/rpc_server.py 1 89.0%
chia/wallet/wallet_node.py 1 88.82%
chia/timelord/timelord_launcher.py 1 69.92%
chia/cmds/dev.py 1 90.0%
chia/cmds/beta_funcs.py 1 86.21%
chia/simulator/simulator_full_node_rpc_api.py 1 96.74%
chia/timelord/timelord.py 2 73.67%
chia/simulator/simulator_full_node_rpc_client.py 2 95.35%
Totals Coverage Status
Change from base Build 8809423530: -0.4%
Covered Lines: 98231
Relevant Lines: 108302

💛 - Coveralls

.pre-commit-config.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

I suppose the super linter wasn't applying this formatting to these files

@altendky
Copy link
Contributor Author

I added some explanation in the OP. This change was inspired by a dependabot update of the super linter.

@altendky altendky closed this Apr 26, 2024
@altendky altendky reopened this Apr 26, 2024
@pmaslana pmaslana merged commit abb0b04 into main Apr 29, 2024
689 of 691 checks passed
@pmaslana pmaslana deleted the move_shfmt_from_superlinter_to_pre_commit branch April 29, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants