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 type-hints from assoc-conversion to work around AOT issue #69

Merged
merged 2 commits into from
Feb 8, 2023

Conversation

DerGuteMoritz
Copy link
Collaborator

Since this function is usually not called at runtime, the reflection performance penalty should be tolerable.

Fixes #68

Since this function is usually not called at runtime, the reflection
performance penalty should be tolerable.

Fixes clj-commons#68
@KingMob
Copy link
Collaborator

KingMob commented Jan 25, 2023

Does it also fix #34?

Copy link
Collaborator

@KingMob KingMob left a comment

Choose a reason for hiding this comment

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

Can you make Eastwood stop complaining about the changes? https://github.com/jonase/eastwood#ignored-faults

@DerGuteMoritz
Copy link
Collaborator Author

Does it also fix #34?

Pretty sure it would (this patch was derived from the workaround in that ticket's description). However, according to #34 (comment) it wasn't reproducible anymore in that particular context so I didn't try anymore.

Can you make Eastwood stop complaining about the changes?

Done!

@KingMob KingMob self-requested a review January 27, 2023 05:32
Copy link
Collaborator

@KingMob KingMob left a comment

Choose a reason for hiding this comment

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

Really wish Eastwood was configurable for names, not just files/lines/cols, but...

@DerGuteMoritz
Copy link
Collaborator Author

Indeed, that's unfortunate 🤷

Note that I don't have merge rights myself here so you'll have to do the honors 🙂

@KingMob
Copy link
Collaborator

KingMob commented Feb 7, 2023

Dammit, let me see about adding you as a contributor

@KingMob
Copy link
Collaborator

KingMob commented Feb 7, 2023

@DerGuteMoritz Try merging now

@DerGuteMoritz DerGuteMoritz merged commit d0c35bb into clj-commons:master Feb 8, 2023
@DerGuteMoritz
Copy link
Collaborator Author

Thanks! 🙏

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.

def-conversion sometimes breaks when AOT-compiled
2 participants