search

open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
779 stars 36 forks source link

docs: add language-specific initialize naming #244

Closed maxveldink closed 9 months ago

maxveldink commented 9 months ago

This PR

Adds callout for language-specific initialize function naming. This is an idea @toddbaert had in this review.

austindrenski commented 9 months ago

This is great, and also lines up with discussions in the dotnet sdk to begin post-fixing spec-defined async method names with Async to match dotnet/BCL conventions.

austindrenski commented 9 months ago

@lukas-reining sounds fine to me. The only question I'd have is if we should still have a suggested name mentioned with the text you quoted? (not sure if there are norms/patterns around this type of thing in other language-agnostic CNCF specs)

lukas-reining commented 9 months ago

@austindrenski I think we do not have to suggest a name. This is as we do in the other cases🤔

toddbaert commented 9 months ago

I think we can avoid a specific name.

Nit: but I recommend you flow the new sentence to the next line, which makes review easier. This doesn't generally impact markdown rendering (though there was a recent regression on openfeature.dev that now includes the source spacing :sweat_smile: ).

maxveldink commented 9 months ago

@toddbaert @lukas-reining Thanks for the suggestions; this is my first spec language PR, so I appreciate the guidance! I like the idea of avoiding the stricter RFC2119 terminology; the goal is to add a simple callout to aid SDK authors. Let me know if that language sounds good!

lukas-reining commented 9 months ago

@maxveldink what do you think of changing the requirement 2.4.1 as stated here to The provider MAY define an initialization function which accepts the global evaluation context as an argument and performs initialization logic relevant to the provider.

I think this is more consistent with the rest of the spec and we do not have to add the note that the name might be changed respecting idioms to every other function that is speced this way. When talking about this function we can just call it initialization function.

maxveldink commented 9 months ago

@lukas-reining I like it! I didn't realize you were referring to changing the actual requirement language. Let me make that update!

toddbaert commented 9 months ago

LGTM! @lukas-reining please merge if you agree.