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

Rename subpackages #5

Merged
merged 4 commits into from
Oct 31, 2022
Merged

Rename subpackages #5

merged 4 commits into from
Oct 31, 2022

Conversation

Southclaws
Copy link
Owner

@Southclaws Southclaws commented Oct 30, 2022

After some feedback from Reddit, discussion on Makeroom and (a small amount of) thinking, I wondered if this is a good compromise to the problem of having lots of differently named sub-packages be passed as arguments to fault.Wrap

    return fault.Wrap(err,
        fault.WithContext(ctx), // decorate the error with key-value metadata from context
        fault.WithKind(kind.NotFound), // categorise the error as a "not found"
        fault.WithIssue("failed to do something", "There was a technical error while retrieving your account"), // provide an end-user message
    )

Was the suggestion, which does read more nicely.

Though I'm conscious of bloating the fault namespace, these are just additional utilities after all and a user of Fault isn't necessarily interested in the error kinds utility or the context utility. For this reason, I would prefer to keep them as sub-packages.

As a compromise, I'm trying out a f- prefix with short names so that they are

  1. easily findable within IDE suggestions (gopls will infer all possible functions that satisfy the decorator signature and when you type f inside fault.Wrap it displays each of the With functions from the Fault utilities)
  2. aligns better and thus reads more smoothly

Now it would look like this:

    return fault.Wrap(err,
        fctx.With(ctx),
        ftag.With(ftag.NotFound),
        fdesc.With("failed to do something", "There was a technical error while retrieving your account"),
    )

Lastly, this is a big nitpick, but I would like fdesc to be 4 characters long, just to align nicely with the others! Maybe I'll think of something tomorrow...

@Southclaws
Copy link
Owner Author

I may remove fault.Msg from the API and instead rename fdesc to fmsg and provide that functionality through there.

// a regular old internal .Error() string message
    return fault.Wrap(err,
        fmsg.With("failed to do something"),
    )

// or with human-readable context

    return fault.Wrap(err,
        fmsg.WithDesc("failed to do something", "There was a technical error while retrieving your account"),
    )

@Southclaws
Copy link
Owner Author

    return fault.Wrap(err,
        fctx.With(ctx),
        ftag.With(ftag.NotFound),
        fmsg.WithDesc("failed to do something", "There was a technical error while retrieving your account"),
    )

perfecto 👌

@Southclaws Southclaws merged commit a68c300 into master Oct 31, 2022
@Southclaws Southclaws deleted the rename-subpackages branch October 31, 2022 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant