Skip to content

feat: Client provider guidelines #14

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

Merged
merged 47 commits into from
May 16, 2025
Merged

feat: Client provider guidelines #14

merged 47 commits into from
May 16, 2025

Conversation

thomaspoignant
Copy link
Member

@thomaspoignant thomaspoignant commented Apr 30, 2024

This PR is a first draft for a provider client spec.

Please provide feedback on this one to know if I am heading in the right direction and to know if it can be helpful to create providers.

Those are the guidelines related to the issue #11

Copy link
Contributor

@markphelps markphelps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple minor typos/grammar suggestions

Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start, thanks! I added a few suggestions but I think this will be helpful.

We may want to avoid calling it a spec (at least for now). I would recommend calling it a guideline.

Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @thomaspoignant for getting started on this 🙌

I have following general remarks,

  • Language - Given this is a spec and to be similar to OF Spec, I think we should generilize langauge to avoid "we" usage (ex:- In this specification we will specify.. to This specificaiton specify..)
  • Spec name & title - Given this is specific to client side (aka static context), a good name for file could be client-provider/ static-context-provider

Otherwise I think this is a great first step

@thomaspoignant thomaspoignant force-pushed the client-spec branch 2 times, most recently from 623950c to 5c18eb8 Compare May 2, 2024 21:18
@thomaspoignant
Copy link
Member Author

Thanks @thomaspoignant for getting started on this 🙌

I have following general remarks,

* Language - Given this is a spec and to be similar to OF Spec, I think we should generilize langauge to avoid "we" usage (ex:- `In this specification we will specify..` to `This specificaiton specify..`)

* Spec name & title - Given this is specific to client side (aka static context), a good name for file could be `client-provider`/ `static-context-provider`

Otherwise I think this is a great first step

I have renamed the file and stopped using the term specification to avoid any confusion here.
Do you think we should continue to update the language if it is only a guideline?

@Kavindu-Dodan
Copy link
Contributor

Kavindu-Dodan commented May 3, 2024

I have renamed the file and stopped using the term specification to avoid any confusion here.

Thanks :)

Do you think we should continue to update the language if it is only a guideline?

Yeah, can start simple and then convert this to a spec. So let's not spend too much time on this.

Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy with this so we can get started :)

@thomaspoignant thomaspoignant changed the title feat: Client provider spec feat: Client provider guidelines May 14, 2024
Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice start and it is really valuable to have!
I left some comments, and I would generally tent to describe the expected behavior of the provider in third person instead of using first person developer.

@thomaspoignant thomaspoignant force-pushed the client-spec branch 2 times, most recently from ed3ac70 to 961f3c7 Compare May 17, 2024 17:09
@Kavindu-Dodan Kavindu-Dodan self-requested a review May 17, 2024 21:26
@thomaspoignant
Copy link
Member Author

I have updated the guidelines with deleting the /ofrep/v1/endpoint.
I'll be interested in any review.

cc @beeme1mr @lukas-reining @jonathannorris @askpt

@thomaspoignant thomaspoignant force-pushed the client-spec branch 2 times, most recently from 6500624 to d8563eb Compare May 8, 2025 10:55
Copy link
Member

@askpt askpt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just a small suggestion but non-blocker.

Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good @thomaspoignant! Left some comments related to wording/style.

thomaspoignant and others added 25 commits May 16, 2025 18:37
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Co-authored-by: Lukas Reining <lukas.reining@codecentric.de>
Signed-off-by: Thomas Poignant <thomas.poignant@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Co-authored-by: Lukas Reining <lukas.reining@codecentric.de>
Signed-off-by: Thomas Poignant <thomas.poignant@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
* feat!: change minPollingInterval field name to mention millisecond

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>

* change name to ms

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>

---------

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
* feat!: Reverse the logic for supportedTypes

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>

* fix

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>

---------

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Honza Dvorsky <honza@apple.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Co-authored-by: Lukas Reining <lukas.reining@codecentric.de>
Signed-off-by: Thomas Poignant <thomas.poignant@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Co-authored-by: Lukas Reining <lukas.reining@codecentric.de>
Signed-off-by: Thomas Poignant <thomas.poignant@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
* feat: Server provider guideline

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>

* adding timeout

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>

---------

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gmail.com>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
@thomaspoignant thomaspoignant merged commit 66f2683 into main May 16, 2025
4 checks passed
@thomaspoignant thomaspoignant deleted the client-spec branch May 16, 2025 16:47
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.