Skip to content

Conversation

@orchestr7
Copy link
Contributor

What's done:

  • Invalid suggestion in warning confuses Kotlin newbies

image

### What's done:
- Invalid suggestion in warning confuses Kotlin newbies
@orchestr7
Copy link
Contributor Author

@ALikhachev as a last contributor in the history :) hey, small friendly ping. Looks like this PR is not so heavy to take :)

@orchestr7
Copy link
Contributor Author

@demiurg906 @Tapchicoma @dimonchik0036 @cypressious tagging you as top contributors for the last month. May be you can give insights on how to merge small PRs like this one into Kotlin? :)

Hate having open PRs in my dashboards

@JSMonk JSMonk requested a review from ilgonmic May 14, 2025 17:05
@JSMonk
Copy link
Member

JSMonk commented May 14, 2025

@orchestr7 Sorry for waiting so long. And thank you for the contribution 🙏
I've asked @ilgonmic to take a look and approve these small changes.

@ilgonmic
Copy link
Contributor

Hi, thank you!
Good catch
LGTM
I could process the review later

Copy link
Member

@dimonchik0036 dimonchik0036 left a comment

Choose a reason for hiding this comment

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

Shouldn't it be covered with tests? A good practice is to have a commit with test data to reproduce the issue, and then a fix for it (if this is applicable for a test).
An example: test + fix

@orchestr7
Copy link
Contributor Author

orchestr7 commented May 15, 2025

Shouldn't it be covered with tests? A good practice is to have a commit with test data to reproduce the issue, and then a fix for it (if this is applicable for a test).

Haha 😄
Then, if it's a good practice, there should be a regression test for this logging?

PS but sure, Dima, I will add it, if needed, np

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