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

add "as" derive helper attribute #174

Merged
merged 12 commits into from
Jan 31, 2024
Merged

Conversation

josema03
Copy link
Contributor

@josema03 josema03 commented Oct 17, 2023

This implements the feature requested in the following issue: #113

Please take a look at it and let me know if I need to make any other change to have it merged.

macros/src/types/named.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@escritorio-gustavo escritorio-gustavo left a comment

Choose a reason for hiding this comment

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

Sorry about the mess, the previous comments were not marking the specific lines I was referring to, but the whole set of changes

macros/src/types/enum.rs Outdated Show resolved Hide resolved
macros/src/types/enum.rs Outdated Show resolved Hide resolved
macros/src/types/enum.rs Outdated Show resolved Hide resolved
macros/src/types/named.rs Outdated Show resolved Hide resolved
macros/src/types/named.rs Outdated Show resolved Hide resolved
macros/src/types/newtype.rs Outdated Show resolved Hide resolved
macros/src/types/newtype.rs Outdated Show resolved Hide resolved
macros/src/types/tuple.rs Outdated Show resolved Hide resolved
@escritorio-gustavo escritorio-gustavo linked an issue Jan 26, 2024 that may be closed by this pull request
@escritorio-gustavo escritorio-gustavo added the enhancement New feature or request label Jan 26, 2024
@escritorio-gustavo
Copy link
Contributor

I have fixed the merge conflict, but I haven't passed the fix through the compiler, needs to be checked

Copy link
Contributor

@escritorio-gustavo escritorio-gustavo left a comment

Choose a reason for hiding this comment

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

I have applied the changes I previously requested. @NyxCode, this PR is a pretty significative change. What do you think?

@NyxCode
Copy link
Collaborator

NyxCode commented Jan 30, 2024

I love it! Thanks @josema03, and thank you so much @escritorio-gustavo for putting in the time to clean this up!
Once it's ready, I think this would be a really cool addition.

@escritorio-gustavo
Copy link
Contributor

escritorio-gustavo commented Jan 30, 2024

Hey @NyxCode!
I'm having some trouble merging this with #213, how should these PRs interact in macros/types/named.rs

Edit: I think I got it working (CI passing) but would still like to have you take another look at this file before merging

@NyxCode
Copy link
Collaborator

NyxCode commented Jan 30, 2024

Hey @NyxCode! I'm having some trouble merging this with #213, how should these PRs interact in macros/types/named.rs

Edit: I think I got it working (CI passing) but would still like to have you take another look at this file before merging

Sure, lemme take a look at that!

macros/src/types/named.rs Outdated Show resolved Hide resolved
@NyxCode
Copy link
Collaborator

NyxCode commented Jan 31, 2024

From my side, this is ready to merge!
@escritorio-gustavo Can you take a last look & merge this? <3

Copy link
Contributor

@escritorio-gustavo escritorio-gustavo left a comment

Choose a reason for hiding this comment

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

LGTM!

@escritorio-gustavo escritorio-gustavo merged commit 3b3fbf8 into Aleph-Alpha:main Jan 31, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workaround for foreign types
3 participants