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

Ignore Kotlin compiler generated annotations during live reload #42363

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

lujun2
Copy link
Contributor

@lujun2 lujun2 commented Aug 7, 2024

Instrument live-reload will compare class structure to make sure this class can be "redefine".

But the classes generated by Kotlin compiler contains debug annotations, the comparison always fail.
Exclude the annotations which generated by Kotlin compiler.

@quarkus-bot quarkus-bot bot added the area/core label Aug 7, 2024
Copy link

quarkus-bot bot commented Aug 7, 2024

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@lujun2 lujun2 changed the title ignore Kotlin compiler generated annotations during live reload Ignore Kotlin compiler generated annotations during live reload Aug 7, 2024
@lujun2 lujun2 force-pushed the fix/kotlin-livereload branch from 8991414 to 594ccc8 Compare August 7, 2024 13:32
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. I added a small suggestion inline.

Comment on lines 24 to 26
static Set<DotName> ignores = new HashSet<>();

static {
ignores.add(DotName.createSimple("kotlin.jvm.internal.SourceDebugExtension"));
ignores.add(DotName.createSimple("kotlin.Metadata"));
}

Copy link
Member

Choose a reason for hiding this comment

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

This could be simplified to the following and made private (and you would need to adjust the name below):

Suggested change
static Set<DotName> ignores = new HashSet<>();
static {
ignores.add(DotName.createSimple("kotlin.jvm.internal.SourceDebugExtension"));
ignores.add(DotName.createSimple("kotlin.Metadata"));
}
private static final Set<DotName> IGNORED_ANNOTATIONS = Set.of(DotName.createSimple("kotlin.jvm.internal.SourceDebugExtension"), DotName.createSimple("kotlin.Metadata"));

@gsmet
Copy link
Member

gsmet commented Aug 7, 2024

LGTM! Could you squash the two commits and force push? I can do it you're not familiar with it. Thanks!

@lujun2 lujun2 force-pushed the fix/kotlin-livereload branch from c783cb5 to a9d3f36 Compare August 8, 2024 00:40
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

@geoand geoand added triage/waiting-for-ci Ready to merge when CI successfully finishes area/kotlin triage/backport labels Aug 8, 2024

This comment has been minimized.

instrument live-reload will compare class structure to make sure this class can be "redefine"
But the classes generated by Kotlin compiler contains debug annotations, the comparison always fail.

Exclude the annotations which generated by Kotlin compiler.
@gsmet gsmet force-pushed the fix/kotlin-livereload branch from a9d3f36 to 28f062d Compare August 8, 2024 10:56
@gsmet
Copy link
Member

gsmet commented Aug 8, 2024

I fixed the formatting issue and force pushed. Let's wait for CI and merge.

@gsmet
Copy link
Member

gsmet commented Aug 8, 2024

I have this alias that is handy to format the Quarkus source code:

alias format="mvn -T 6 process-sources"

Copy link

quarkus-bot bot commented Aug 8, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 28f062d.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand geoand merged commit b4f76a1 into quarkusio:main Aug 8, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Aug 8, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 8, 2024
@gsmet gsmet modified the milestones: 3.14 - main, 3.13.2 Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quarkus instrument live-reload doesn't work when using Kotlin 1.8+
3 participants