SaturnFramework / Saturn

Opinionated, web development framework for F# which implements the server-side, functional MVC pattern
https://saturnframework.org
MIT License
715 stars 109 forks source link

Discussion: naming convention CE operations #89

Closed NinoFloris closed 2 years ago

NinoFloris commented 6 years ago

Currently we use snakecase for the CEs but camelCase is the normal F# convention, I'm all for it and we didn't hit 1.0 yet so ¯_(ツ)/¯ Also it's already inconsistent looking at the subController op.

/cc: @isaacabraham @rmunn

rmunn commented 6 years ago

I think the most relevant thing here is this part of the F# component design guidelines (emphasis mine):

Use either PascalCase or camelCase for public functions and values in F# modules

camelCase is used for public functions that are designed to be used unqualified (for example, invalidArg), and for the “standard collection functions” (for example, List.map). In both these cases, the function names act much like keywords in the language.

There's no mention of custom operations in CEs here, but I'd say that things like pipe_through and use_github_oauth are definitely designed to "act much like keywords in the language". So to best fit .Net naming schemes, I think I'm also in favor of switching to camelCase instead of snake_case for these. It's definitely a breaking change for ALL existing code, of course, so perhaps a deprecation is needed, e.g.:

    ///`signOff` signs off the currently logged in user.
    [<CustomOperation("signOff")>]
    member __.SignOff(state, scheme) : HttpHandler  = state >=> (signOut scheme)

    ///`sign_off` is deprecated and should be replaced with `signOff`.
    [<CustomOperation("sign_off")>]
    [<Obsolete("Use signOff instead")>]
    member __.SignOffObsolete(state, scheme) : HttpHandler  = __.SignOff(state, scheme)

It would get tedious doing this for every custom operation in the project, but creating the __.FooObsolete method that just calls __.Foo with the same parameters is something one might be able to do semi-automatically with some clever use of macros in an editor, or even VS Code's multiple-cursors function (do a regex search for \[<CustomOperation\(.+_.+\)>\], hit F1 and choose "Select All Occurrences of Find Match", then use some clever cursor movement to select just the custom operation name... something like that.

But that's beside the point. The point is that although snake_case is very idiomatic in Python and, judging from the Phoenix documentation, in Elixir, it's not idiomatic in .Net. So although Saturn was very much inspired by Phoenix, I think it would be best for Saturn to follow .Net naming conventions, hence replacing everything with camelCase.

Krzysztof-Cieslak commented 6 years ago

Am I only one who likes current naming convention? ;-)

forki commented 6 years ago

Yes.

Krzysztof Cieślak notifications@github.com schrieb am Mo., 9. Juli 2018, 11:27:

Am I only one who likes current naming convention? ;-)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/SaturnFramework/Saturn/issues/89#issuecomment-403417775, or mute the thread https://github.com/notifications/unsubscribe-auth/AADgNC3G2g3IYtVS7Rd504PV6rABztQYks5uEyIcgaJpZM4VFW1I .

Krzysztof-Cieslak commented 6 years ago

forki commented 6 years ago

Hugs

Krzysztof Cieślak notifications@github.com schrieb am Mo., 9. Juli 2018, 14:28:

https://camo.githubusercontent.com/6526c158b2d7bdd58107e81d69a2e32e5281a5ab/68747470733a2f2f6d656469612e67697068792e636f6d2f6d656469612f796f4a43324f6c7830656b4d79326e5837572f67697068792d646f776e73697a65642e676966

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/SaturnFramework/Saturn/issues/89#issuecomment-403461880, or mute the thread https://github.com/notifications/unsubscribe-auth/AADgNHOV1blBly9mRaN9Z3N8VwXyMNB6ks5uE0xjgaJpZM4VFW1I .

MarneeDear commented 5 years ago

Am I only one who likes current naming convention? ;-)

I prefer snake case, myself.

NinoFloris commented 5 years ago

Can we pick one before 1.0? (obsolete names that lost, introduce proper aliases for them, done)

panesofglass commented 5 years ago

Just to make it more fun and provide a (most likely :trollface:) "no", how about:

    ///```sign off``` signs off the currently logged in user.
    [<CustomOperation("sign off")>]
    member __.SignOff(state, scheme) : HttpHandler  = state >=> (signOut scheme)

That would appear as:

``sign off``
Krzysztof-Cieslak commented 4 years ago

Have we all agreed that the sneak case is the best way? ;P

rmunn commented 4 years ago

Well, this comment suggesting camelCase got 5 upvotes, while this comment suggesting snake_case got 1 favorite (from you), so it looks to me like a 6-to-2 vote for camelCase over snake_case. Though since you're the maintainer, you could assign your vote a score of 100 if you want... ;-)

panesofglass commented 4 years ago

It would be easy enough to extend the CE with an opt-in library like SaturnFramework.FixedCEs that adds the camelCase names. ;-)

rmunn commented 4 years ago

It would be easy enough to extend the CE with an opt-in library like SaturnFramework.FixedCEs that adds the camelCase names. ;-)

I like it! This allows continuing the current snake_case naming convention, which means minimal changes to existing code, and also means @Krzysztof-Cieslak will be happy. :-) And it also allows people who prefer camelCase to have that option, in an officially-supported way. The SaturnFramework.CamelCase (or CamelCaseCEs or FixedCEs, though I admit I don't care for the FixedCEs name as it's not evident from the name how they're "fixed") library would have to be part of the official Saturn framework release rather than being a third-party library, otherwise there's too much danger of code drift.

panesofglass commented 4 years ago

@rmunn The "FixedCEs" name was merely a joke. 😉

tforkmann commented 2 years ago

Farmer is also using a lot of snake_case naming in CE. I got pretty much used to that… @isaacabraham Why did you used snake_case?