feat: introduce spotless/ktlint #39
Conversation
whitescent
requested review from
eymar and
JSMonk
and removed request for
eymar
|
But we may need to write a Gradle task to download the latest compose rules ktlint jar from https://github.com/mrmans0n/compose-rules/releases during project synchronization and import it into the project, so that the Ktlint IDEA plugin can import lint suggestions related to compose?
|
| @@ -8,7 +8,10 @@ import org.jetbrains.compose.storytale.example.Res | |||
| import org.jetbrains.compose.storytale.example.compose_multiplatform | |||
|
|
|||
| @Composable | |||
| fun ComposeLogo(size: LogoSize = LogoSize.MEDIUM) { | |||
| fun ComposeLogo( | |||
| modifier: Modifier = Modifier, | |||
There was a problem hiding this comment.
Hmm. Was this suggested by spotless?
The problem here is that both modifier can set sizes and size argument too. So they will conflict. And size set in modifier would be ignored.
size: LogoSize = LogoSize.MEDIUM is a very specific parameter, while Modifier can be very flexible. I'd personally prefer it to be as more specific as possible in this case.
Do you know which rule exactly forces this?
There was a problem hiding this comment.
Ah, this rule was originally mandatory, but I have disabled it. This change might have been made before I disabled the rule. If we want to keep the modifier parameter, what are some good suggestions in this situation?
There was a problem hiding this comment.
To be honest, it's a difficult question :)
On one hand, we want to provide a list of "popular" parameters, especially if they need to have default values. It would simplify the usage.
One another hand, when the parameters can conflict with modifiers - it's a problem, because the actual behaviour can be identified only by reading the code or the documentation if it's available.
Let's say we wan't the Logo to be only in 3 sizes. But with Modifier it's possible to set any size.
In this case, I guess it's not so important, because it's just an example project - not a library of UI components.
| @@ -29,36 +29,36 @@ import org.jetbrains.compose.storytale.gallery.ui.theme.currentColorScheme | |||
| fun StoryListItem( | |||
| story: Story, | |||
| selected: Boolean, | |||
| onClick: () -> Unit, | |||
There was a problem hiding this comment.
I'm wondering what rule caused this change :)
Usually the trailing lambda indeed is the last parameter. But not in this case for some reason?
There was a problem hiding this comment.
settings.gradle.kts
Outdated
| @@ -30,6 +30,7 @@ dependencyResolutionManagement { | |||
| } | |||
| } | |||
|
|
|||
| // include(":examples") | |||
There was a problem hiding this comment.
Sorry, no, because to run Storytale in the examples module, it is necessary to include(":examples"). I forgot to remove it
|
Hi @whitescent ! |
.editorconfig
Outdated
|
|
||
| [*] | ||
| charset = utf-8 | ||
| indent_size = 2 |
There was a problem hiding this comment.
Can we use 4 spaces, as suggested by the official convention - https://kotlinlang.org/docs/coding-conventions.html#indentation ?
There was a problem hiding this comment.
Yes! but for the compose project, I might prefer 2 spaces for indentation, as it looks more readable in cases with deep component nesting. However, if 4 spaces are to be used, that's fine too
There was a problem hiding this comment.
Got it.
One more argument for keeping it 4 is that the repository is in Kotlin group, so using the official convention is preferred :)
#37 #36