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 support for top level type overrides #286

Merged
merged 6 commits into from
Mar 28, 2024

Conversation

dr-bonez
Copy link
Contributor

@dr-bonez dr-bonez commented Mar 27, 2024

Goal

This adds support for easily defining a struct or enum's type definition manually at the top level using the type = ".." attribute. This is useful for when you have a custom serialize/deserialize and don't want to manually implement the TS trait.

Changes

Added type_override to the container attributes, and defined an early-returned implementation for if this attribute is specified

Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made.

@NyxCode
Copy link
Collaborator

NyxCode commented Mar 28, 2024

I like this! It's a simple change and it's cleanly implemented!
What do you think @escritorio-gustavo?

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 two small nitpicks, otherwise, LGTM

macros/src/types/mod.rs Outdated Show resolved Hide resolved
macros/src/types/type_override.rs Show resolved Hide resolved
@escritorio-gustavo
Copy link
Contributor

@NyxCode is there anything else you'd like to change?

Copy link
Collaborator

@NyxCode NyxCode left a comment

Choose a reason for hiding this comment

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

Nothing from my side!

Great work @dr-bonez, thanks!

@NyxCode NyxCode merged commit 8b77622 into Aleph-Alpha:main Mar 28, 2024
7 checks passed
@dr-bonez dr-bonez mentioned this pull request Apr 1, 2024
3 tasks
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