Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fixed property from interface implemented by trait - #1487

#1487
Merged
Ocramius merged 1 commit into
Roave:6.57.xRoave/BetterReflection:6.57.xfrom
ondrejmirtes:trait-interface-propertyondrejmirtes/BetterReflection:trait-interface-propertyCopy head branch name to clipboard
Feb 12, 2025
Merged

Fixed property from interface implemented by trait#1487
Ocramius merged 1 commit into
Roave:6.57.xRoave/BetterReflection:6.57.xfrom
ondrejmirtes:trait-interface-propertyondrejmirtes/BetterReflection:trait-interface-propertyCopy head branch name to clipboard

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Contributor

No description provided.

@ondrejmirtes

Copy link
Copy Markdown
Contributor Author

Days without bugs in property hooks: 0
Days without bugs in traits: 0

@ondrejmirtes

Copy link
Copy Markdown
Contributor Author

The problem here is that BR looks for the property in interface first and completely skips the implementation in the used trait.

@kukulich
kukulich force-pushed the trait-interface-property branch from 45952b9 to 94b7e42 Compare February 12, 2025 20:07
@kukulich kukulich changed the title Failing test - property from interface implemented by trait Fixed property from interface implemented by trait Feb 12, 2025
@kukulich kukulich added the bug label Feb 12, 2025
@kukulich kukulich self-assigned this Feb 12, 2025
@kukulich
kukulich force-pushed the trait-interface-property branch from 94b7e42 to fedbd2a Compare February 12, 2025 20:08
@kukulich
kukulich marked this pull request as ready for review February 12, 2025 20:09
@kukulich
kukulich force-pushed the trait-interface-property branch from fedbd2a to 8f5e710 Compare February 12, 2025 20:16

@Ocramius Ocramius left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Noice!

I mean, it's still a monstrosity, but the fix is nice.

@Ocramius Ocramius added this to the 6.57.0 milestone Feb 12, 2025
@Ocramius
Ocramius merged commit d5fa8e1 into Roave:6.57.x Feb 12, 2025
@Ocramius

Copy link
Copy Markdown
Member

Thanks @kukulich!

@ondrejmirtes

Copy link
Copy Markdown
Contributor Author

Thank you very much!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.