-
Notifications
You must be signed in to change notification settings - Fork 7
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
v1.0 Roadmap - consolidate built-in wrappers and implement API and DX improvements. #40
Comments
Hi! Thanks for the ping I'd advise against single letter imports as those are usually taken by variables ( Why not just go all-in and get rid of the subpackage entirely? Just create some functional options that may or not be passed into Example: return fault.Wrap(err,
fault.WithCtx(ctx), // decorate the error with key-value metadata from context
fault.WithTag(fault.TagNotFound), // categorise the error as a "not found"
fault.WithMsg("There was a technical error while retrieving your account"), // provide an end-user message
) fault is almost there already, it's a matter of just moving some code up to the root. This would only increase the API surface from 7 to 10+ symbols + the leftover types from those subpackages that are needed (like |
One of the main reasons I wanted to keep the wrappers separate was to emphasise that Fault itself is standalone and the real value it provides is the I agree with the single letter point though, that's something I didn't consider! Brevity has its pros and cons, too short and the meaning is lost, too long and it becomes arduous to use and read. Perhaps return fault.Wrap(err,
wrap.Ctx(ctx),
wrap.Tag(fault.TagNotFound),
wrap.Msg("There was a technical error while retrieving your account"),
) |
Just my 2 cents, but if we're talking about 1.0 then I would consider the following. It seems to me that we could provide more generic version of I feel it would provide far greater possibility for customizing fault for specific use cases. If you think this is something worth pursuing, I could give it a try and come up with a first draft an a possible API and we could iterate on it. |
It's been about a year that this library has been in production use in multiple real-world revenue-generating products and I've got a good handle on the improvements I want to make and the direction this library is taking.
So, to mark this, I'd like to bundle the existing wrappers into a single package for a few reasons:
Unwrap() []error
interfaceWrap(New(...), wrappers...)
justNew("msg", wrappers...)
🥳I'd like to use a single nice short package name for all the built-in wrappers, I'm thinking of just
fault/f
orfault/w
. Mostly just for aesthetics, similar to how Zap and other log aggregators work:Settled on
wrap
for context, tags and user-friendly error message reference wrappersPrevious ideas:
Or
with
:Or justw
toqueteos pointed out this would be a bad idea as single letter symbols are often used for variable and parameter names.
Any ideas for this are welcome though! cc @matdurand @the-goodies @semihbkgr @toqueteos (just tagging folks who have contributed in case you have ideas or reservations around this!)
The text was updated successfully, but these errors were encountered: