-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: add errorz package to combine errors as enum in there #47
feat: add errorz package to combine errors as enum in there #47
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a branch with some suggestions, you can have a look at them here 8c42502
and incorparate them in your code as you see fit.
As discussed with MF on friday, I used a sligtly more discriptive module name and would suggest you do the same here.
If you have questions, I am available
@bmo-at-a9s I check you suggestions and I think your suggestion totally makes sense to me so no objection. I guess we can merge your changes into this branch. |
I test your suggestions but are you sure that this changes pass tests?
|
Yeah, the test case needed to be adjusted for this to work with our new dynamic helper error. |
I just created an errorz package that will contain errors enum so we can use them multiple time and change them in one place.
I was thinking about adding couple of helper functions but I don't know that much the requirements of project. But place is ready to write sth.