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

Expose internal errors. #1387

Merged
merged 1 commit into from
Jun 16, 2021
Merged

Conversation

jorgii
Copy link
Contributor

@jorgii jorgii commented Jun 16, 2021

Please provide enough information so that others can review your pull request:

This pull request solves exposes the internal errors so that they can be used publicly.

Explain the details for making this change. What existing problem does the pull request solve?

The problem is described in #1375.

Commit formatting

Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/

@welcome
Copy link

welcome bot commented Jun 16, 2021

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87 ReneWerner87 linked an issue Jun 16, 2021 that may be closed by this pull request
@jorgii
Copy link
Contributor Author

jorgii commented Jun 16, 2021

This is the easy version of the fix, aliasing the internal errors. This brings the potential problem where someone working on the internal error might not realise that they are exposed to the public.
I am pushing this as a start.
This could be improved by moving all the error definitions outside so that it is explicit they are public.

Does that make sense to do?

@jorgii jorgii marked this pull request as draft June 16, 2021 08:40
@ReneWerner87
Copy link
Member

yes it makes sense, but because this internal code is a copy, it will be difficult to do so

@jorgii
Copy link
Contributor Author

jorgii commented Jun 16, 2021

Should I try and do the refactor?

@ReneWerner87
Copy link
Member

but when we refresh the code we copied, this customization flies out again, so i think the current solution is good

@jorgii
Copy link
Contributor Author

jorgii commented Jun 16, 2021

Alright, making the PR ready for merge.

@jorgii jorgii marked this pull request as ready for review June 16, 2021 08:58
@ReneWerner87 ReneWerner87 merged commit 1852224 into gofiber:master Jun 16, 2021
@welcome
Copy link

welcome bot commented Jun 16, 2021

Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🤗 How to handle errors from internal package?
2 participants