-
Notifications
You must be signed in to change notification settings - Fork 73
Writing kdocs+tests for generateCode.kt #1311
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
base: master
Are you sure you want to change the base?
Conversation
) | ||
|
||
@Language("kotlin") | ||
val expected = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are very similar tests on CodeGenerator itself. If you want to test generateDataClasses specifically, maybe you can compare results to results of CodeGenerator?
like df.generateInterfaces should be CodeGenerator.generateCode(...)
So we have less code Strings to refactor should anything change.. or, what do you think?
* | ||
* For more information: {@include [DocumentationUrls.DataSchemaGeneration]} | ||
* | ||
* @return [CodeString] – A value class wrapper for [String], containing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and CodeString.print()
utility :)
public fun <T> DataFrame<T>.generateInterfaces(markerName: String): CodeString = | ||
schema().generateInterfaces(markerName = markerName) | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please mention that data classes can be used with "toListOf"
interface UseFqNames | ||
|
||
/** | ||
* @param nameNormalizer Strategy for converting column names (with spaces, underscores, etc.) to valid Kotlin identifiers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to use word "Kotlin-style identifiers" for default
normalizer
And
"Generated properties will still refer to columns by their actual name via @ColumnName annotation`
/** @include [MarkerName] If not specified, generates a name from type [T]. */ | ||
interface MarkerNameOptional | ||
|
||
/** @param fields Whether to generate fields (`val ...:`) inside the generated data schema declarations (markers). */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to put it.. Technically field = false, extensions = true is enough to refer to columns in dataframe
But plugin won't be able to extract schema from such dataschema. At least we should state it, at best maybe even remove this option?
/** @param fields Whether to generate fields (`val ...:`) inside the generated data schema declarations (markers). */ | ||
interface Fields | ||
|
||
/** @param extensionProperties Whether to generate extension properties in addition to data schema declarations (markers). */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we should say that if either compiler plugin or ksp are used in the project, extension properties will conflict with generated ones.
Soo, same as with fields, does it even make sense to have this option..
interface ExtensionProperties | ||
|
||
/** @param visibility Visibility modifier for the generated declarations (markers). */ | ||
interface Visibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add some context, this argument was added to generate valid code for https://kotlinlang.org/docs/api-guidelines-simplicity.html#use-explicit-api-mode
Maybe it's a useful info, maybe not :)
Fixes #1281