Skip to content

[BREAKING] Fix TrustedOrigins default issue - #327

Open
saoirse-a wants to merge 3 commits into
eclipse-biscuit:mainfrom
saoirse-a:trusted-origin-default
Open

[BREAKING] Fix TrustedOrigins default issue#327
saoirse-a wants to merge 3 commits into
eclipse-biscuit:mainfrom
saoirse-a:trusted-origin-default

Conversation

@saoirse-a

Copy link
Copy Markdown
Contributor

TrustedOrigins implements an inherent constructor called default and also has a derived implementation of Default. These two constructors have different behavior.

This means that TrustedOrigins::default() and <TrustedOrigins as Default>::default() construct different TrustedOrigins types. This is surprising.

This PR changes the behavior of the Default trait constructor to match the behavior of the inherent constructor.

@divarvel divarvel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh, nice catch

@divarvel

Copy link
Copy Markdown
Contributor

Could you add an entry in the changelog as well?

@saoirse-a

Copy link
Copy Markdown
Contributor Author

Added to the changelog under 7.0.0

@divarvel

divarvel commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Since #334 is moving on, I guess we have a strong case for a major release

@divarvel

divarvel commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@saoirse-a I’ll merge #334 first, can you rebase this one on top of main afterwards? I’ll merge it then

TrustedOrigins implements an inherent constructor called default and also
has a derived implementation of Default. **These two constructors have
different behavior.**

This means that `TrustedOrigins::default()` and `<TrustedOrigins as
Default>::default()` construct different `TrustedOrigins` types. This is
surprising.

This PR changes the behavior of the Default trait constructor to match
the behavior of the inherent constructor.
@saoirse-a
saoirse-a force-pushed the trusted-origin-default branch from e101081 to 334dc55 Compare August 11, 2026 12:38
@saoirse-a

Copy link
Copy Markdown
Contributor Author

rebased

@saoirse-a

Copy link
Copy Markdown
Contributor Author

Actually I also updated it to delete the inherent default constructor; since Default is in the prelude this shouldn't break anything, there's no reason to have it.

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