sdk_csharp icon indicating copy to clipboard operation
sdk_csharp copied to clipboard

Published 20 hours ago • verified publisher icon bunq

Rename constants to PascalCase

Open epels opened this issue 7 years ago • 8 comments
trafficstars

Steps to reproduce:

  1. Read the source code

What should happen:

  1. Constants are named in PascalCase

What happens:

  1. Constants are named IN_ALL_CAPS.

See also:

https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/capitalization-conventions

Implementations

  • [ ] Core code
  • [ ] Generated code

epels avatar Dec 21 '17 11:12 epels

@epels @OGKevin did a quick look into 'const' keyword in the source and could not find any that was not in pascal case. Did I missed something or is this issue closed?

Thanks!

kid-cavaquinho avatar Dec 27 '17 09:12 kid-cavaquinho

@antao I dont quite understand ?

Currently all constants are in CAPS, which is not correct according to the style guide in the OP. So the source code must be refactored to rename the constants in PascalCase as the style guide says!

OGKevin avatar Dec 27 '17 09:12 OGKevin

@OGKevin my bad, you are right! 👍

kid-cavaquinho avatar Dec 27 '17 10:12 kid-cavaquinho

To contribute you can follow this guide: https://guides.github.com/activities/forking/

The constants in https://github.com/bunq/sdk_csharp/tree/develop/BunqSdk/Model/Generated should not be changed as these must be changed on bunq’s end. As these files are generated.

Thanks for helping out 👍

On 27 Dec 2017, at 14:17, João Antão [email protected] wrote:

Hi again, @OGKevin https://github.com/ogkevin . I would like to start contributing to bunq oss for several reasons... I have a small contribution to start regarding this issue. But I cannot push the feature branch towards github. (I am getting a 403) Do I need special permissions?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bunq/sdk_csharp/issues/58#issuecomment-354112613, or mute the thread https://github.com/notifications/unsubscribe-auth/ARGTBpq1AacP0QVKGlgNOYYyPw7snem-ks5tEkOFgaJpZM4RJrfN.

OGKevin avatar Dec 27 '17 13:12 OGKevin

Thanks for the input @OGKevin , closed the other PR's cause they did not followed the naming conventions. Hopefully in the next days I will look further into the codebase 👍

kid-cavaquinho avatar Dec 27 '17 15:12 kid-cavaquinho

Due to this introducing a lot of breaking change, this will be moved to 0.13.0 release instead of 0.12.5`

OGKevin avatar Dec 28 '17 13:12 OGKevin

@antao We can still finish up the PR tho!

OGKevin avatar Dec 28 '17 13:12 OGKevin

@antao I think the constants in the non generated files are almost done 👏 🎊 👍.

The ones in the generated folder, ill take care of them once 0.13.0 is near as this requires change in the generator and will introduce breaking change. With this being said, lets keep this on hold for now and finish up the 0.12.5 milestone.

OGKevin avatar Dec 28 '17 20:12 OGKevin