Skip to content

fix: implement uuid format for string#72

Merged
asyncapi-bot merged 4 commits intoasyncapi:masterfrom
dan-r:uuid
Nov 2, 2022
Merged

fix: implement uuid format for string#72
asyncapi-bot merged 4 commits intoasyncapi:masterfrom
dan-r:uuid

Conversation

@dan-r
Copy link
Member

@dan-r dan-r commented Oct 24, 2022

Description

Implementing UUID format for string type

Related issue(s)
#23

@dan-r dan-r changed the title Implement uuid format for string fix: Implement uuid format for string Oct 24, 2022
@dan-r dan-r changed the title fix: Implement uuid format for string fix: implement uuid format for string Oct 24, 2022
@dan-r
Copy link
Member Author

dan-r commented Oct 24, 2022

Just realised this conflicts with @dalelane's latest changes, let me rework...

return 'String';
case 'password':
return 'String';
case 'uuid':
Copy link
Collaborator

Choose a reason for hiding this comment

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

uuid isn't a format in the AsyncAPI spec but it's commonly used enough that it seems reasonable to add

We'll need to update the template to add java.util.UUID to the imports for any class that uses it, though - as it's not a primitive

Copy link
Member Author

Choose a reason for hiding this comment

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

@dalelane worth importing java.util.UUID regardless or doing it conditionally?

Copy link
Collaborator

Choose a reason for hiding this comment

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

depends on how much effort it would be, I guess - it'd be nice to not add unused imports, but it wouldn't cause errors so it's not worth a massive amount of work, IMO

Copy link
Member Author

Choose a reason for hiding this comment

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

I've just imported it for now, but I think a future task is to cleanup how we do this, as its likely future datatypes will make this more of an issue

Copy link
Collaborator

@dalelane dalelane left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@dan-r
Copy link
Member Author

dan-r commented Nov 2, 2022

/rtm

@asyncapi-bot asyncapi-bot merged commit f71b2e7 into asyncapi:master Nov 2, 2022
@dan-r dan-r mentioned this pull request Nov 2, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants