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

refactor: parrot message types #198

Merged
merged 11 commits into from
May 29, 2022
Merged

Conversation

joao-conde
Copy link
Collaborator

- -
Issue #123
Dependencies --
Decisions In the world of rust and typed languages, we should make more usage of types. Sending ad-hoc strings as messages (mythical strings.rs) worked fine but we can do better. It becomes a bigger problem now due to #123 where we want to have mappings of translations. This means if I want to send the HelloWorld message I can do so in fr, en, or pt. If all I use is a string, I have no way of knowing which message it is and how to translate it. This PR preps the translations PR.
Here we introduce the concept of message types. These types are then displayed. A future message translation implementation will take into account the server's set language and return a different string to display.
Another important topic is error handling. I did it in a way where every error was bubbled up. While intended at the time, I see now its a bit weird for the user to receive messages like "invalid token" or something of the sorts. A next PR will make the automatic conversion from ParrotError -> ParrotMessage -> sent to user. This way, we can filter specific error messages we want to display and convert those into a specific ParrotMessage and for the other errors, we just throw a generic error message.

@joao-conde joao-conde added the 🧼 refactor Refactors to existing code label May 29, 2022
@joao-conde joao-conde requested review from aquelemiguel and afonsojramos and removed request for aquelemiguel May 29, 2022 12:24
Copy link
Collaborator

@afonsojramos afonsojramos left a comment

Choose a reason for hiding this comment

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

Looking good!! Way cleaner now! Quality PR 💯

@afonsojramos afonsojramos merged commit fd3548d into main May 29, 2022
@afonsojramos afonsojramos deleted the jc/refactor-parrot-response-types branch May 29, 2022 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧼 refactor Refactors to existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants