Add generated comments to Kotlin files (default true) #486
Conversation
Will respect the existing `--omit-file-comments` flag Fixes microsoft#485
| @@ -2606,5 +2617,12 @@ class KotlinCodeGenerator( | |||
| private fun String.capitalize(): String { | |||
| return this.replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.US) else it.toString() } | |||
| } | |||
|
|
|||
| companion object { | |||
| private const val FILE_COMMENT = | |||
There was a problem hiding this comment.
Not sure how you feel about this copypasta; there wasn't an obvious shared place to put the constants but I'm happy to move.
There was a problem hiding this comment.
I'd have imagined that after working in this codebase, you'd have seen enough copypasta from me not to worry about it 😆
(This is absolutely fine by me, thank you for asking)
| private fun generate(thrift: String, config: (KotlinCodeGenerator.() -> KotlinCodeGenerator)? = null): List<FileSpec> { | ||
| val configOrDefault = config ?: { this } | ||
| val configOrDefault = config ?: { emitFileComment(false) } |
There was a problem hiding this comment.
I can also default this to true but it's a bit annoying to handle the changing timestamp.
Codecov Report
@@ Coverage Diff @@
## master #486 +/- ##
============================================
+ Coverage 59.15% 59.18% +0.02%
- Complexity 865 868 +3
============================================
Files 69 69
Lines 6481 6488 +7
Branches 1010 1012 +2
============================================
+ Hits 3834 3840 +6
- Misses 2345 2346 +1
Partials 302 302
Continue to review full report at Codecov.
|
| @@ -149,6 +151,7 @@ class KotlinCodeGenerator( | |||
| private var emitJvmName: Boolean = false | |||
| private var emitJvmStatic: Boolean = false | |||
| private var emitBigEnums: Boolean = false | |||
| private var emitFileComment: Boolean = true | |||
There was a problem hiding this comment.
Personally I'd prefer this to default to false, if only to minimize the disruption to others. AFAIK most people using thrifty are still manually checking in the generated code.
There was a problem hiding this comment.
@benjamin-bader I don't have a strong opinion either way, but it does default to true for Java sources, which means the default will change depending on whether you generate Java or Kotlin. I will defer to you.
There was a problem hiding this comment.
You make a good point. I'd forgotten entirely about the Java code-generator, despite directly referring to it. Shows where our focus is, I suppose. Let's keep it on by default, then.
|
Sorry for the delay; crazy family visit == no time to think about anything! I appreciate the effort. This basically LGTM but I'd like to make the comment an opt-in feature - that is, it should default to |
|
@benjamin-bader How dare you prioritize family over your volunteer OSS work! :) Anyway, I responded to your comment re the default value. I can update the docs if we end up having different behavior for Java vs Kotlin. |
It's one weakness that I indulge, I admit. Thanks for your contributions, as always! |
Will respect the existing
--omit-file-commentsflagFixes #485