Unify API methods to check for attribute (GH-731)#1095
Unify API methods to check for attribute (GH-731)#1095dennisdoomen merged 2 commits intofluentassertions:masterfrom
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 |
| 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.
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
XXXOrInheritnotXXXOrInheritsas recommended to follow the convention used by already implemented methods.Some of the tests are failing because of my changes on
PropertyInfoSelector.csbut 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.