-
Notifications
You must be signed in to change notification settings - Fork 37
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
configureJsonApi overload (formatter can be only add at end of the collection, without delete other formatters #168
Conversation
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.
Looks good! I like the backwards compatibility. I guess we should not have made that a bool
to begin with.
A few naming/phrasing things could be slightly clearer.
Saule/ConfigureFormattersEnum.cs
Outdated
/// <summary> | ||
/// Configuration enum for formatters | ||
/// </summary> | ||
public enum ConfigureFormattersEnum |
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.
Maybe FormatterPriority
?
Saule/ConfigureFormattersEnum.cs
Outdated
namespace Saule | ||
{ | ||
/// <summary> | ||
/// Configuration enum for formatters |
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.
This description isn't very clear. Maybe describe the purpose rather than what it does?
Saule/Http/HttpConfigExtensions.cs
Outdated
/// <param name="config">The <see cref="HttpConfiguration"/> that is used in the setup of the application.</param> | ||
/// <param name="jsonApiConfiguration">Configuration parameters for Json Api serialization.</param> | ||
/// <param name="configureFormatters"> | ||
/// AddFormatterToStart - Formatter will be inserted at the start of the collection. |
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.
This should be pretty clear from the enum values; maybe something like Determines the relative position of the JSON API formatter
?
Thanks for this! I will publish a pre-release shortly with these changes in it. |
I added more options for configure formatters in ConfigureJsonApi. For example if want combinate JSON API with WebAPI together.
#167