Skip to content

Conversation

@nevakrien
Copy link
Contributor

so similar to the ideas in https://github.com/zesterer/ariadne/pull/99/files but without breaking any old code.
this version should just solve #40 with no down sides

Copy link
Owner

@zesterer zesterer left a comment

Choose a reason for hiding this comment

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

I think this is a good addition. The only improvement I can imagine is to make the implementation of the old impl call out to the implementation of the new impl, removing the code duplication.

@nevakrien
Copy link
Contributor Author

nevakrien commented Nov 27, 2025

that improvement cant quite work because of lifetimes. the other way around works and i have added it.
tried to figure out if there might be something nicer we could do but nothing comes to mind

@nevakrien nevakrien requested a review from zesterer November 27, 2025 23:34
@zesterer
Copy link
Owner

Thanks, that's just as good! It seems like it just needs a cargo fmt, and then should be good to merge :)

@nevakrien
Copy link
Contributor Author

@zesterer fixed

@zesterer
Copy link
Owner

Thanks!

@nevakrien
Copy link
Contributor Author

@zesterer, so is there anything else left? Or is this just waiting for a feature window or something?

@zesterer
Copy link
Owner

zesterer commented Dec 1, 2025

No, I just fell asleep while CI was running 😄

@zesterer zesterer merged commit 8be691f into zesterer:main Dec 1, 2025
3 checks passed
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.

2 participants