Skip to content
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

Drop exception handler in for tag to reduce bytecode size #224

Merged
merged 2 commits into from
Jun 14, 2023

Conversation

e5l
Copy link
Member

@e5l e5l commented Jun 9, 2023

No description provided.

@e5l e5l requested a review from anton-bannykh June 9, 2023 06:57
@e5l e5l self-assigned this Jun 9, 2023
@e5l e5l changed the title Drop exception handler in block to reduce bytecode size Drop exception handler in for tag to reduce bytecode size Jun 9, 2023
Copy link

@rsinukov rsinukov left a comment

Choose a reason for hiding this comment

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

LGTM

}
}

repositories {
mavenCentral()
}



Choose a reason for hiding this comment

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

unnecessary empty lines

@e5l e5l merged commit f1ca2c6 into master Jun 14, 2023
@languitar
Copy link

Seems this change got into the 0.9.0 release without being somehow marked as a breaking change. For us, this has effectively broken error handling in a ktor project. Is this expected?

@e5l
Copy link
Member Author

e5l commented Jul 19, 2023

@languitar, yep, it is. Could you share the case of how exceptions were handled in your project?

@languitar
Copy link

What was broken was the integration with kotlinx html provided by ktor themselves. But it seems the vanished method is handled on main already: https://youtrack.jetbrains.com/issue/KTOR-6124/Not-compatible-with-kotlinx-html-0.9.1

Is it intended that breaking changes appear in this project without major version bumps?

@e5l
Copy link
Member Author

e5l commented Jul 19, 2023

Yep, it is before the stable 1.0.0release. Now we assume only patch releases safe

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.

3 participants