Skip to content

Squash annotations#1647

Closed
koperagen wants to merge 2 commits into
masterfrom
squash-annotations
Closed

Squash annotations#1647
koperagen wants to merge 2 commits into
masterfrom
squash-annotations

Conversation

@koperagen

Copy link
Copy Markdown
Collaborator

I thought, let's employ a bit shorted syntax to reduce noise
Maybe there are other annotations that can be squashed, i squashed only the most popular

@koperagen koperagen added this to the 1.0.0-Beta5 milestone Dec 18, 2025
@koperagen koperagen self-assigned this Dec 18, 2025
Comment thread core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/filter.kt
@Jolanrensen

Jolanrensen commented Dec 18, 2025

Copy link
Copy Markdown
Collaborator

I would try to follow/create a little style guide for all our annotations. I just submitted an issue for ktlint ktlint/ktlint#3212 for this, as it doesn't seem to know @[] notations.

You now create:

@[Deprecated(DEPRECATED_ACCESS_API) AccessApiOverload]
@Suppress("INAPPLICABLE_JVM_NAME")
@JvmName("frameColDataFrameKProperty")
public fun <C> AnyColumnGroupAccessor.frameCol...

I would recommend putting the @[] at the bottom. It looks a bit cleaner, in my opinion:
You now create:

@Suppress("INAPPLICABLE_JVM_NAME")
@JvmName("frameColDataFrameKProperty")
@[Deprecated(DEPRECATED_ACCESS_API) AccessApiOverload]
public fun <C> AnyColumnGroupAccessor.frameCol...

Also, we don't really honor an order. What about just lexicographically?

@JvmName("frameColDataFrameKProperty")
@Suppress("INAPPLICABLE_JVM_NAME")
@[AccessApiOverload Deprecated(DEPRECATED_ACCESS_API)]
public fun <C> AnyColumnGroupAccessor.frameCol...

Next, the plan is to "compact" multiple annotations that are often used together?

We could probably include the JvmName one then too:

@[AccessApiOverload Deprecated(DEPRECATED_ACCESS_API)]
@[JvmName("frameColDataFrameKProperty") Suppress("INAPPLICABLE_JVM_NAME")]
public fun <C> AnyColumnGroupAccessor.frameCol...

wdyt?

@AndreiKingsley

Copy link
Copy Markdown
Collaborator

@Jolanrensen to be honest, it doesn't look very pretty to me, however the idea of grouping annotations seems great!

@zaleslaw

zaleslaw commented Feb 10, 2026

Copy link
Copy Markdown
Collaborator

Honestly, I'd prefer the next order:

@Suppress("INAPPLICABLE_JVM_NAME")
@JvmName("frameColDataFrameKProperty") 
  • at the bottom, because they are services and do not give so much to developers, the most significant things, like deprecation, should be at the top, and this is how it's usually added by IDE, isn't it?

@zaleslaw zaleslaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry, I want to argue with this approach

Although @[A B] is valid Kotlin syntax, it is not a conventional style. It is rarely used outside of use-site targets, is not shown in official Kotlin documentation examples, and is not commonly found in real Kotlin projects or codebases. It is also not adopted by common tools or JetBrains/Android codebases.

In this case, it replaces the familiar form

@Deprecated(...)
@AccessApiOverload

with

@[Deprecated(...) AccessApiOverload]

which mixes a standard Kotlin annotation with a project-specific one, reduces readability, makes diffs harder to read, and introduces an uncommon idiom without any clear benefit.

Moreover, it's absolutely JVM, Kotlin and other approaches have the list of these aspects of the function as separate annotations

@koperagen

Copy link
Copy Markdown
Collaborator Author

We agreed to not follow up on this idea

@koperagen koperagen closed this Feb 10, 2026
@Jolanrensen

Copy link
Copy Markdown
Collaborator

Let's return to it once the community reaches a consensus on what is recommended annotation notation, like with KtLint: ktlint/ktlint#3212

Personally, it makes sense for me to group annotations that are related in some way, but it should be done consistently everywhere as to keep the codebase clean and predictable :)

@koperagen koperagen deleted the squash-annotations branch March 9, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants