Skip to content

Conversation

CarloMariaProietti
Copy link
Contributor

@CarloMariaProietti CarloMariaProietti commented Sep 25, 2025

About issue #1431

@Jolanrensen Jolanrensen added the enhancement New feature or request label Sep 29, 2025
@Jolanrensen
Copy link
Collaborator

Jolanrensen commented Sep 29, 2025

Great addition! Thanks for including KDocs and tests too!

I see .all {} also has an overload on DataColumn<T>, could you do the same for .none {} while you're at it? :) that will save us work in the future

@Jolanrensen Jolanrensen changed the title adding none to data frame adding .none {} to DataFrame Sep 29, 2025
@CarloMariaProietti
Copy link
Contributor Author

CarloMariaProietti commented Sep 29, 2025

I'll try to do the same for .none {} . Should I open a new PR for this?

@Jolanrensen
Copy link
Collaborator

I'll try to do the same for .none {} . Should I open a new PR for this?

I think it's fine if they're in the same PR as they're siblings anyway :) thanks!

@Jolanrensen
Copy link
Collaborator

please run ktlintFormat to check everything is formatted right and apiDump, as you added a new public function :) Then I'll run our CI on the branch and we should be good to go :)

@Jolanrensen Jolanrensen added this to the 1.0.0-Beta4 milestone Sep 30, 2025
Copy link
Collaborator

@AndreiKingsley AndreiKingsley left a comment

Choose a reason for hiding this comment

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

Great, thank you!

@CarloMariaProietti
Copy link
Contributor Author

I ran ktlintFormat but i don't know about apiDump

@Jolanrensen
Copy link
Collaborator

I ran ktlintFormat but i don't know about apiDump

alright, I'll do it for you this time. The idea is that you run the linter with ktlintFormat and commit the changes. It takes care of formatting and unused imports etc. apiDump updates the .api file of our module to check binary compatibility across versions (with apiCheck). This should also be run and committed for PRs :)

@Jolanrensen
Copy link
Collaborator

The CI agrees :) I'll merge it, thanks!

@Jolanrensen Jolanrensen merged commit 56b76f4 into Kotlin:master Oct 1, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants