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
Unify API methods to check for attribute (GH-731) #1095
Unify API methods to check for attribute (GH-731) #1095
Conversation
Hmm, the code you touched is supposed to be used internally and should not have been |
51ac7d6
to
75f717a
Compare
@dennisdoomen Which strategy do you want to go for? Breaking change, in which case we can change the accessibility, current option (with |
@@ -65,7 +66,18 @@ public PropertyInfoSelector ThatAreDecoratedWith<TAttribute>() | |||
public PropertyInfoSelector ThatAreDecoratedWithOrInherit<TAttribute>() | |||
where TAttribute : Attribute | |||
{ | |||
selectedProperties = selectedProperties.Where(property => CustomAttributeExtensions.GetCustomAttributes(property, true).OfType<TAttribute>().Any()); | |||
// TODO: There seems to be a bug with `GetCustomAttributes` when used as extension method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that there is a bug with GetCustomAttributes
, I'll file an issue on Microsoft side.
@Evangelink If I remember correct |
I think we can make all of these |
@jnyrup it seems that the bug only happens when using the method as extension method so I did a change in the code and left a message to explain why, @dennisdoomen Just to be sure, are you referring to |
All the methods of the |
Do you want me to do it in this PR or to create a new one? |
Whatever you like ;-) |
@dennisdoomen Are you okay with |
ping @dennisdoomen |
I like the PR as it is right now. |
I'll try to push a rebase before the week-end. Thank you for the review :) |
c128bc1
to
233ddfa
Compare
233ddfa
to
ac06323
Compare
@dennisdoomen @jnyrup PR ready to be merged! |
Fix #731
I have marked the old methods as obsolete but depending on your preferences, I could get rid of them (breaking changes).
I decided to name the methods
XXXOrInherit
notXXXOrInherits
as recommended to follow the convention used by already implemented methods.Some of the tests are failing because of my changes on
PropertyInfoSelector.cs
but I can't figure out what's wrong with my change. I'd be grateful for some fresh eyes because I am likely missing something obvious.