Skip to content
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

Custom enum description attribute property names #277

Merged

Conversation

mexx
Copy link
Collaborator

@mexx mexx commented May 23, 2014

Resolves #272
any feedback is welcome

@MehdiK
Copy link
Member

MehdiK commented May 23, 2014

Thanks @mexx. That was quick!! The TC build is broken. Can you check that out please?

@@ -98,5 +100,20 @@ public static IDateTimeHumanizeStrategy DateTimeHumanizeStrategy
get { return _dateTimeHumanizeStrategy; }
set { _dateTimeHumanizeStrategy = value; }
}

private const string DefaultEnumDescriptionPropertyName = "Description";
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about the implementation. It feels a bit complex. What if we just created a DescriptionPropertyLocator predicate function? Let them keep the logic. This way they can be as flexible as they want!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to provide the possibility to have different property names per enum type.
But as already mentioned it a first attempt, so let discuss further the possibilities.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After a second thought, it would be flexible enough since the PropertyInfo has the reference to the DeclaringType, so even my use case can be implemented with the predicate function.

I'll rewrite it now.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if that level of granular control is necessary to be honest with you. Also remember PropertyInfo is from the attribute not the target enum!

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can just expose Func<PropertyInfo, bool>? That way they won't be able to nominate different attributes for different enums but I call YAGNI on it for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you're right. Func<PropertyInfo, bool> is done.

MehdiK added a commit that referenced this pull request May 23, 2014
…roperty-names

Custom enum description attribute property names
@MehdiK MehdiK merged commit d41dc18 into Humanizr:master May 23, 2014
@MehdiK
Copy link
Member

MehdiK commented May 23, 2014

Awesome. Thanks @mexx

@mdmoura
Copy link

mdmoura commented May 23, 2014

Thanks! I will give it a try and provide feedback ...

@MehdiK
Copy link
Member

MehdiK commented May 23, 2014

Not deployed yet. I will let you know as soon as it hits NuGet.

On Fri, May 23, 2014 at 6:50 PM, Miguel Moura [email protected]:

Thanks! I will give it a try and provide feedback ...


Reply to this email directly or view it on GitHubhttps://github.com//pull/277#issuecomment-44068204
.

@MehdiK
Copy link
Member

MehdiK commented Jun 22, 2014

This is now released to NuGet as v1.27.0. Thanks for your contribution.

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.

Humanize Custom Attribute
3 participants