Reformat Kotlin code to upgrade `ktlint` version #4488

ks-yim · 2022-10-18T11:42:11Z

Motivation:

To upgrade ktlint version from 0.36.0 to ktlint-gradle plugin's default (0.43.2).
Armeria's some kotlin code violates the new ktlint version's default ruleset, especially, when it comes to adding newlines.

Modifications:

Run ./gradlew ktlintFormat task once.
Manually reformat some code for better readability.

codecov · 2022-10-18T12:52:23Z

Codecov Report

Base: 74.05% // Head: 74.03% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (eac8378) compared to base (8bc6f19).
Patch coverage: 7.31% of modified lines in pull request are covered.

❗ Current head eac8378 differs from pull request most recent head 7823909. Consider uploading reports for the commit 7823909 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #4488      +/-   ##
============================================
- Coverage     74.05%   74.03%   -0.02%     
+ Complexity    18167    18159       -8     
============================================
  Files          1536     1536              
  Lines         67378    67380       +2     
  Branches       8520     8520              
============================================
- Hits          49894    49885       -9     
- Misses        13412    13423      +11     
  Partials       4072     4072

Impacted Files	Coverage Δ
...rp/armeria/client/ClientRequestContextWrapper.java	`8.10% <0.00%> (ø)`
...linecorp/armeria/common/RequestContextWrapper.java	`9.75% <0.00%> (-2.44%)`	⬇️
...ver/annotated/kotlin/MarkdownDescriptionService.kt	`29.41% <0.00%> (ø)`
...rver/annotated/kotlin/MermaidDescriptionService.kt	`66.66% <ø> (ø)`
...e/armeria/contextpropagation/kotlin/MainService.kt	`0.00% <0.00%> (ø)`
...p/armeria/server/ServiceRequestContextWrapper.java	`9.43% <2.77%> (ø)`
...rmeria/internal/common/kotlin/ArmeriaKotlinUtil.kt	`39.47% <31.25%> (+3.36%)`	⬆️
...n/java/com/linecorp/armeria/server/HttpServer.java	`40.00% <0.00%> (-20.00%)`	⬇️
...armeria/internal/common/stream/SubscriberUtil.java	`72.22% <0.00%> (-11.12%)`	⬇️
.../linecorp/armeria/client/retrofit2/PipeBuffer.java	`84.44% <0.00%> (-4.45%)`	⬇️
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ks-yim · 2022-10-24T00:50:03Z

@minwoox @ikhoon @jrhee17
I've updated the PR to reformat kotlin code as per the discussion in line/gradle-scripts#134.
PTAL 🙏. It would be nice if you can give some advice about the right process of updating line-gradle-scripts.

minwoox · 2022-10-24T01:52:04Z

Thanks for updating it. 😄

It would be nice if you can give some advice about the right process of updating line-gradle-scripts.

We use git-subrepo.
https://github.com/line/armeria/blob/master/gradle/scripts/.gitrepo#L4
The process is:

Sending a PR to /line/armeria to check if it works.
Sending a PR to /line/gradle-scripts that contains the changes of Gradle scripts.
Merging the gradle-scripts PR.
Updating the Armeria PR using git-subrepo. (In the root directory, you can just execute git subrepo pull gradle/scripts)
Merging and Done.

ikhoon

Thanks!! 🙇‍♂️

jrhee17

Looks good! Left a minor question. Thanks for the cleanup @ks-yim 🙇 👍 🙇

jrhee17 · 2022-11-01T06:56:56Z

gradle/scripts/lib/kotlin.gradle

@@ -27,7 +25,7 @@ configure(projectsWithFlags('kotlin')) {
            disabledRules = ["import-ordering"]

            filter {
-                exclude("**/gen-src/**")
+                exclude { it.file.path.contains("gen-src/") }


Minor Question) What kind of patterns does the updated rule filter as opposed to the original pattern?

Not sure of the exact reason but the project I've recently setup won't build well without adding this directive.
It tries to lint the files in project-root/gen-src/main/kotlinGrpc/*.

The ktlint plugin states that this can happen for dynamically attaches sources that are located outside the project dir (ref. https://github.com/JLLeitschuh/ktlint-gradle#faq).

I will make a reproducer and share it here.

That's interesting. Thanks for fixing it. 😄

I will make a reproducer and share it here.

Because it's obvious and the link clearly explains the limit, I think we don't need to check the reproducer unless you really want to share. 😆

@jrhee17 @minwoox
Sorry I was late 😅. Made a reproducer at https://github.com/ks-yim/gradle-scripts-grpc-kotlin-ktlint-error-reproducer.
The build fails with the following error.

> Task :grpc-app:compileJava Note: /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/java/com/google/protobuf/compiler/PluginProtos.java uses or overrides a deprecated API. Note: Recompile with -Xlint:deprecation for details. > Task :grpc-app:ktlintMainSourceSetCheck FAILED /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:1:1 object HelloServiceGrpcKt should be declared in a file named HelloServiceGrpcKt.kt (cannot be auto-corrected) (filename) /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:27:1 Unexpected indentation (2) (should be 4) (indent) /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:29:1 Unexpected indentation (2) (should be 4) (indent) /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:30:1 Unexpected indentation (2) (should be 4) (indent) /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:31:1 Unexpected indentation (4) (should be 8) (indent) /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:33:1 Unexpected indentation (2) (should be 4) (indent) /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:34:1 Unexpected indentation (4) (should be 8) (indent) /Users/ks-yim/IdeaProjects/gradle-scripts-ktlint/grpc-app/gen-src/main/kotlinGrpc/HelloGrpcKt.kt:35:1 Unexpected indentation (4) (should be 8) (indent) ...

minwoox · 2022-11-28T11:03:11Z

line/gradle-scripts#134
line/gradle-scripts#135
are merged. Could you pull the changes using subrepo, please?

subrepo: subdir: "gradle/scripts" merged: "10ef4cfa8" upstream: origin: "https://github.com/line/gradle-scripts" branch: "master" commit: "10ef4cfa8" git-subrepo: version: "0.4.5" origin: "https://github.com/ingydotnet/git-subrepo" commit: "dbb99be"

ks-yim · 2022-11-28T13:03:07Z

@minwoox
Pulled the new master!
But I couldn't just do git subrepo pull gradle/scripts and succeeded after adding -f flag.

It looks like the original parent SHA in .gitrepo file no longer exists.

minwoox · 2022-12-02T08:22:32Z

@ks-yim, thanks soooooo much! 😄

Test line-gradle-script update

fca8517

ks-yim requested review from ikhoon, jrhee17, minwoox and trustin as code owners October 18, 2022 11:42

ks-yim mentioned this pull request Oct 18, 2022

Fix jvmTarget & ktlint config in kotlin flag preset line/gradle-scripts#134

Merged

ks-yim added 4 commits October 23, 2022 17:27

reformat kotlin code

04b7322

manual reformat some kotlin code

4f20588

Merge remote-tracking branch 'upstream/master' into test-gradle-scripts

a3268a0

manual reformat - 2

eac8378

ks-yim changed the title ~~[TEST] line-gradle-script update~~ Reformat Kotlin code to upgrade ktlint version Oct 24, 2022

ikhoon approved these changes Oct 31, 2022

View reviewed changes

ikhoon added the cleanup label Oct 31, 2022

ikhoon added this to the 1.21.0 milestone Oct 31, 2022

jrhee17 approved these changes Nov 1, 2022

View reviewed changes

minwoox merged commit 75867ae into line:master Dec 2, 2022

ks-yim deleted the test-gradle-scripts branch December 2, 2022 11:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reformat Kotlin code to upgrade `ktlint` version #4488

Reformat Kotlin code to upgrade `ktlint` version #4488

ks-yim commented Oct 18, 2022 •

edited

Loading

codecov bot commented Oct 18, 2022 •

edited

Loading

ks-yim commented Oct 24, 2022

minwoox commented Oct 24, 2022

ikhoon left a comment

jrhee17 left a comment

jrhee17 Nov 1, 2022

ks-yim Nov 4, 2022

minwoox Nov 28, 2022

ks-yim Nov 28, 2022

minwoox commented Nov 28, 2022

ks-yim commented Nov 28, 2022

minwoox commented Dec 2, 2022

Reformat Kotlin code to upgrade ktlint version #4488

Reformat Kotlin code to upgrade ktlint version #4488

Conversation

ks-yim commented Oct 18, 2022 • edited Loading

codecov bot commented Oct 18, 2022 • edited Loading

Codecov Report

ks-yim commented Oct 24, 2022

minwoox commented Oct 24, 2022

ikhoon left a comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

jrhee17 Nov 1, 2022

Choose a reason for hiding this comment

ks-yim Nov 4, 2022

Choose a reason for hiding this comment

minwoox Nov 28, 2022

Choose a reason for hiding this comment

ks-yim Nov 28, 2022

Choose a reason for hiding this comment

minwoox commented Nov 28, 2022

ks-yim commented Nov 28, 2022

minwoox commented Dec 2, 2022

Reformat Kotlin code to upgrade `ktlint` version #4488

Reformat Kotlin code to upgrade `ktlint` version #4488

ks-yim commented Oct 18, 2022 •

edited

Loading

codecov bot commented Oct 18, 2022 •

edited

Loading