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

Add ktlint plugin code styling and formatting #281

Merged
merged 12 commits into from
Oct 21, 2023

Conversation

G10xy
Copy link
Contributor

@G10xy G10xy commented Oct 12, 2023

Description

Based on the issue #279 I already used ktlint plugin, in order to mantain a standard about code style and formatting.
Please have check, give it a try.
You can find in the Grandle task at the sub-task formatting.
By running ktlintFormat, the code will be automatically restyled and, in case of .* imports, it will generate a txt file reporting the error.

Some link about:
https://reflectoring.io/code-format-with-ktlint/
https://www.baeldung.com/kotlin/ktlint-code-formatting

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Dependency update / replacement

How Has This Been Tested?

I run ktlintFormat and checked that it reported all problems about importing and automatically formatting the code style.
I also tested it with gradle check and gradle build and it worked fine everytime.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation if necessary
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #281 (267636a) into master (2fa6004) will decrease coverage by 0.15%.
Report is 1 commits behind head on master.
The diff coverage is 39.13%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #281      +/-   ##
============================================
- Coverage     35.88%   35.74%   -0.15%     
  Complexity      348      348              
============================================
  Files           101      101              
  Lines          3035     3047      +12     
  Branches        184      184              
============================================
  Hits           1089     1089              
- Misses         1826     1838      +12     
  Partials        120      120              
Files Coverage Δ
...e/src/main/kotlin/social/bigbone/JsonSerializer.kt 100.00% <100.00%> (ø)
...igbone/src/main/kotlin/social/bigbone/api/Range.kt 100.00% <100.00%> (ø)
...n/kotlin/social/bigbone/api/entity/Conversation.kt 36.36% <100.00%> (ø)
...rc/main/kotlin/social/bigbone/api/entity/Filter.kt 57.69% <ø> (ø)
...rc/main/kotlin/social/bigbone/api/entity/Marker.kt 44.44% <100.00%> (ø)
...n/kotlin/social/bigbone/api/entity/MastodonList.kt 61.53% <100.00%> (ø)
...ain/kotlin/social/bigbone/api/entity/StatusEdit.kt 27.58% <100.00%> (ø)
...kotlin/social/bigbone/api/method/AccountMethods.kt 75.94% <100.00%> (ø)
...n/social/bigbone/api/method/FeaturedTagsMethods.kt 94.73% <ø> (ø)
...n/kotlin/social/bigbone/nodeinfo/NodeInfoClient.kt 33.33% <ø> (ø)
... and 9 more

@andregasser
Copy link
Owner

andregasser commented Oct 15, 2023

Hello @G10xy
Thanks a lot for this PR. As we are already using detekt (with detekt-formatting enabled) in this project, we should either continue using detekt-formatting or switch this off and start using ktlint instead.

detekt wraps ktlint formatting rules already:

image

Reference: https://detekt.dev/docs/intro

Not sure how to continue with this. At the moment, I'm having an issue that I want detekt to alert on wildcard imports. But somehow, I cannot make this work. Eventually, this works better with pure ktlint?

Best regards,
André

bigbone/build.gradle Outdated Show resolved Hide resolved
bigbone/build.gradle Outdated Show resolved Hide resolved
@andregasser
Copy link
Owner

andregasser commented Oct 15, 2023

After doing a bit of research, I think we should keep detekt as a tool for code analysis, but use ktlint Gradle plugin for linting.
Especially after reading this: detekt/detekt#5997

I have added two minor comments to your PR. Would be great, if you could fix this.
Also, we need to disable detekt-formatting entirely, which means, we need to remove the declarations of it from various places:

  • Gradle version catalog file (gradle/libs.versions.toml)
  • Gradle kotlin convention file (buildSrc/src/main/groovy/bigbone.kotlin-conventions.gradle), lines 7-9
  • Remove the formatting rules from the detekt config file (config/detekt/detekt.yml)

detekt as a tool should remain in place, we just want to remove the detekt-formatting functionality.

# Conflicts:
#	bigbone/build.gradle
#	bigbone/src/main/kotlin/social/bigbone/api/entity/Conversation.kt
#	bigbone/src/main/kotlin/social/bigbone/api/entity/Marker.kt
#	bigbone/src/main/kotlin/social/bigbone/api/entity/MastodonList.kt
#	bigbone/src/main/kotlin/social/bigbone/api/entity/MediaAttachment.kt
#	bigbone/src/main/kotlin/social/bigbone/api/entity/PreviewCard.kt
#	bigbone/src/main/kotlin/social/bigbone/api/entity/Report.kt
#	bigbone/src/main/kotlin/social/bigbone/api/entity/StatusEdit.kt
@G10xy
Copy link
Contributor Author

G10xy commented Oct 15, 2023

After doing a bit of research, I think we should keep detekt as a tool for code analysis, but use ktlint Gradle plugin for linting. Especially after reading this: detekt/detekt#5997

I have added two minor comments to your PR. Would be great, if you could fix this. Also, we need to disable detekt-formatting entirely, which means, we need to remove the declarations of it from various places:

* Gradle version catalog file (`gradle/libs.versions.toml`)

* Gradle kotlin convention file (`buildSrc/src/main/groovy/bigbone.kotlin-conventions.gradle`), lines 7-9

* Remove the formatting rules from the detekt config file (`config/detekt/detekt.yml`)

detekt as a tool should remain in place, we just want to remove the detekt-formatting functionality.

Thanks, I honestly did not know so deeply about.

@andregasser andregasser added this to the 2.0.0 milestone Oct 15, 2023
@PattaFeuFeu PattaFeuFeu added code quality Everything related to code quality pipeline CI/CD topics labels Oct 16, 2023
@PattaFeuFeu PattaFeuFeu self-assigned this Oct 16, 2023
Copy link
Owner

@andregasser andregasser left a comment

Choose a reason for hiding this comment

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

Everything looks good! Thank you very much 👍

@andregasser andregasser merged commit 5dcaea0 into andregasser:master Oct 21, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Everything related to code quality hacktoberfest-accepted pipeline CI/CD topics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants