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

ci: Add pre-commit yaml configuration #17227

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mishomihov00
Copy link
Contributor

@mishomihov00 mishomihov00 commented Jan 6, 2025

Description:

Add pre-commit yaml configuration for the pre-commit tool, which will run the spotlessApply gradle task.

Related issue(s):

Fixes #17223

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@mishomihov00 mishomihov00 added the DevOps DevOps dependency, review, test and deployment. label Jan 6, 2025
@mishomihov00 mishomihov00 added this to the v0.59 milestone Jan 6, 2025
@mishomihov00 mishomihov00 self-assigned this Jan 6, 2025
@mishomihov00 mishomihov00 requested review from a team as code owners January 6, 2025 14:31
@mishomihov00 mishomihov00 linked an issue Jan 6, 2025 that may be closed by this pull request
Copy link

codacy-production bot commented Jan 6, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (be1981d) 96191 68558 71.27%
Head commit (13f7f9a) 96191 (+0) 68558 (+0) 71.27% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#17227) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Jan 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.41%. Comparing base (be1981d) to head (13f7f9a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #17227   +/-   ##
=========================================
  Coverage     67.41%   67.41%           
  Complexity    22063    22063           
=========================================
  Files          2585     2585           
  Lines         96408    96408           
  Branches      10071    10071           
=========================================
  Hits          64992    64992           
  Misses        27698    27698           
  Partials       3718     3718           

Impacted file tree graph

Copy link
Member

@nathanklick nathanklick left a comment

Choose a reason for hiding this comment

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

We should offer a pre-commit.yaml configuration instead of building a custom git hook.

@mishomihov00
Copy link
Contributor Author

@nathanklick The idea here is to make it the same way as it is in hedera-mirror-node: https://github.com/hashgraph/hedera-mirror-node/blob/a6da57c2e94e65859a2c4e39cca0620836128cde/buildSrc/build.gradle.kts#L58

@mustafauzunn Can also provide more details if needed.

@rbarkerSL
Copy link
Contributor

@nathanklick The idea here is to make it the same way as it is in hedera-mirror-node: https://github.com/hashgraph/hedera-mirror-node/blob/a6da57c2e94e65859a2c4e39cca0620836128cde/buildSrc/build.gradle.kts#L58

@mustafauzunn Can also provide more details if needed.

I think @nathanklick s point is that we shouldn't be modifying local commit history with this pre-commit hook. Offering the pre-commit yaml and the gradle task which already exists ./gradlew spotlessApply should be enough. And then the workflows will block any PR that fails spotless.

@jjohannes
Copy link
Collaborator

It would be good to reach some general agreement on how/if such a feature should exist in all Hedera repositories.

When we move MirrorNode to the shared Gradle configuration – hashgraph/hedera-mirror-node#10035 – the Gradle part has to be addressed. Either by removing the functionality or by adding something that works everywhere to the shared configuration.

My feeling is that it would be best to do this, if at all, without Gradle being involved in installing the hook. E.g., if some developers would like to use such a hook they can install it on their machine manually. And we just host the hook setup for calling spotlessApply (https://github.com/hashgraph/hedera-mirror-node/blob/a6da57c2e94e65859a2c4e39cca0620836128cde/buildSrc/src/main/resources/hooks/pre-commit) somewhere central for folks to use it if desired.

@mishomihov00 mishomihov00 force-pushed the 17223-add-a-pre-commit-hook-for-spotlessapply-1 branch from 0879c57 to 6a69965 Compare January 9, 2025 09:17
@mishomihov00 mishomihov00 changed the title ci: Add spotlessApply pre-commit hook auto install with gradle ci: Add pre-commit yaml configuration Jan 9, 2025
@mishomihov00 mishomihov00 force-pushed the 17223-add-a-pre-commit-hook-for-spotlessapply-1 branch from b42e01d to 4a0db08 Compare January 10, 2025 07:06
@mishomihov00 mishomihov00 force-pushed the 17223-add-a-pre-commit-hook-for-spotlessapply-1 branch from 4a0db08 to 13f7f9a Compare January 10, 2025 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DevOps DevOps dependency, review, test and deployment.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a pre-commit hook for spotlessApply
4 participants