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

Add ability to set default naming convention #120

Merged

Conversation

Odonno
Copy link
Contributor

@Odonno Odonno commented Mar 24, 2024

Add default naming convention property so that we can simply set a default naming convention that will be apply on every field like this:

var options = new CborOptions
{
    DefaultNamingConvention = new SnakeCaseNamingConvention(),
};

It will still be overriden by the use of CborNamingConventionAttribute.

@mcatanzariti
Copy link
Member

Hello,

Could you check 373266e, please?
It should already implement what you want.

Thank you,

Michaël

@Odonno
Copy link
Contributor Author

Odonno commented Mar 25, 2024

Hello Michaël,

I don't understand this.

First, I don't understand the unecessary complexity of this commit. Isn't it more simple to pass a default naming policy? Secondly, it expect to pass a Type where you can simply pass a a INamingConvention which IMO is way better (in term of usability and maintainability).

Also, what is the point of Lowercase and Uppercase convention? My end goal is to have feature parity with JsonNamingConvention which mean having the following policies:

  • default = null (PascalCase)
  • CamelCase
  • LowerSnakeCase (or pure SnakeCase)
  • UpperSnakeCase
  • LowerKebabCase (or pure KebabCase)
  • UpperKebabCase

Furthermore, I will add tests to prevent regression.

@mcatanzariti
Copy link
Member

Hi @Odonno,

I understand & I agree with you.
Please can you merge with your modifications and implement what you suggested?

Thank you <3

@Odonno Odonno force-pushed the feat/default-naming-convention branch from 79a0866 to a3e3881 Compare March 27, 2024 11:02
@Odonno
Copy link
Contributor Author

Odonno commented Mar 27, 2024

  • Passing an object instead of a Type for default naming convention
  • Added missing naming convention like explicited, to have feature parity with JsonNamingPolicy
  • Updated and added some tests

I let the Lower and Upper conventions because they may be useful for some.. If you don't see the point of those, I'll let you remove the code.

@mcatanzariti mcatanzariti merged commit 1964845 into dahomey-technologies:master Mar 27, 2024
2 checks passed
@Odonno Odonno deleted the feat/default-naming-convention branch March 27, 2024 15:36
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.

2 participants