Skip to content

Make WellKnownError public#598

Merged
spacebear21 merged 2 commits intopayjoin:masterfrom
spacebear21:pub-well-known-error
Mar 21, 2025
Merged

Make WellKnownError public#598
spacebear21 merged 2 commits intopayjoin:masterfrom
spacebear21:pub-well-known-error

Conversation

@spacebear21
Copy link
Collaborator

Well known errors are required for ffi bindings and should be public

@spacebear21 spacebear21 requested a review from DanGould March 21, 2025 18:04
@coveralls
Copy link
Collaborator

coveralls commented Mar 21, 2025

Pull Request Test Coverage Report for Build 13998660299

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 80.411%

Totals Coverage Status
Change from base Build 13955812638: 0.03%
Covered Lines: 4848
Relevant Lines: 6029

💛 - Coveralls

@DanGould
Copy link
Contributor

IMO well known error should be #[non_exhaustive] before it's public. It's unclear whether or not there will be other protocol error variants for fee negotiation

It's unclear whether or not there will be other protocol error variants
for fee negotiation.
Well known errors are required for ffi bindings and should be public
@spacebear21 spacebear21 force-pushed the pub-well-known-error branch from f8d9591 to 5e871ac Compare March 21, 2025 18:39
@spacebear21
Copy link
Collaborator Author

Good point, added. Do you think ResponseError should also be non_exhaustive?

@DanGould
Copy link
Contributor

I can't think of an error that's not well known, unrecognized, or a validation. i don't think ResponseError needs to be non_exhaustive.

Copy link
Contributor

@DanGould DanGould left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 5e871ac

@spacebear21 spacebear21 merged commit 2e57272 into payjoin:master Mar 21, 2025
7 checks passed
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.

3 participants