-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: ♻️ simplify handling of grouped errors #81
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
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.
Nice! Question: Is the title "demo" mean anything? Like, you don't want this to be merged in or?
We can merge it in if you like this approach! I just wasn't sure if anyone had a better idea for dealing with all the custom mapping. There will be 15 cases all in all, so that will require lots of rather specific and arcane code. |
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.
Nice progress! Some more feedback
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.
Great progress!! 🎉 some more comments
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.
Awesome!! Just some very minor comments
Co-authored-by: Luke W. Johnston <lwjohnst86@users.noreply.github.com>
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.
🎉 Amazing 🎉
Description
This PR refactors how the grouped errors listed in #15 could be simplified.
It adds custom logic for the first 2 items in the list.
We will probably be able to share quite a bit of code between the various error cases, this is just a start.
Needs an in-depth review.
Checklist
just run-all