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

docs: update progressive rollout docs #2456

Merged

Conversation

Light2Dark
Copy link
Contributor

@Light2Dark Light2Dark commented Oct 6, 2024

Description

Update the example used and add a description of how the configuration works. (updated)
Screenshot 2024-10-06 at 6 46 12 PM

Closes issue(s)

Resolve #2452

Checklist

  • I have tested this code
  • I have added unit test to cover this code
  • I have updated the documentation (README.md and /website/docs)
  • I have followed the contributing guide

Copy link

netlify bot commented Oct 6, 2024

Deploy Preview for go-feature-flag-doc-preview ready!

Name Link
🔨 Latest commit f394983
🔍 Latest deploy log https://app.netlify.com/sites/go-feature-flag-doc-preview/deploys/67026a53f2e6e90008e3bca0
😎 Deploy Preview https://deploy-preview-2456--go-feature-flag-doc-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Light2Dark Light2Dark marked this pull request as draft October 6, 2024 07:47
@Light2Dark Light2Dark changed the title remove old docs for progressive rollout and update example docs: update progressive rollout docs Oct 6, 2024
@Light2Dark
Copy link
Contributor Author

Hey @thomaspoignant , I faced this issue and wanted to contribute to the docs. I think it might be worth to explain a little more on changing the percentage field. However, it is unintuitive. The following is my proposal to be added to the Example section:

Using the percentage field (advanced)

If you do not intend to completely rollout a feature, you can modify the percentage field.

    progressiveRollout:
      initial:
        variation: variationA
        percentage: 20   
        date: 2024-01-01T05:00:00.100Z
      end:
        variation: variationB
        percentage: 80
        date: 2024-01-05T05:00:00.100Z
  • Before the initial date, the flag will return variationB 20% of the time and variationA 80% of the time.
  • Between the initial and end date, the flag will progressively change from returning variationA to variationB.
  • After the end date, the flag will return variationB 80% of the time and variationA 20% of the time.

Copy link

codecov bot commented Oct 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.56%. Comparing base (a680901) to head (f394983).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2456   +/-   ##
=======================================
  Coverage   85.56%   85.56%           
=======================================
  Files         103      103           
  Lines        3816     3816           
=======================================
  Hits         3265     3265           
  Misses        425      425           
  Partials      126      126           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@thomaspoignant thomaspoignant left a comment

Choose a reason for hiding this comment

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

@Light2Dark thanks a lot for this improvement of the documentation this is really great.

I agree that your proposal for the example is better to understand how it works, just to be sure you want to have 2 different examples? One without percentage `(which means that we go from 0 to 100) and one with it?
I like your proposal, the bullet list helps a lot to understand better how it works.

The only thing I would change is the Configuration fields table that is not reflecting the format, I would prefer to have an entry for initial and one for end.

| **`percentage`** | *(optional)*<br/>It represents the ramp of progress, at which level the flag starts (`initial`) and ends (`end`).<br/>**Default: `initial` = `0` and `end` = `100`** |
| Field | Description |
|------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| **`percentage`**<br/>*(optional)* | It represents the ramp of progress, at which level the flag starts (`initial`) and ends (`end`).<br/>**Default: `initial` = `0` and `end` = `100`**. |
Copy link
Owner

Choose a reason for hiding this comment

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

We probably need to change this table with explaining what should be in initial and end.

@Light2Dark
Copy link
Contributor Author

Light2Dark commented Oct 6, 2024

I agree that your proposal for the example is better to understand how it works, just to be sure you want to have 2 different examples? One without percentage `(which means that we go from 0 to 100) and one with it? I like your proposal, the bullet list helps a lot to understand better how it works.

Yeap, 2 examples. One is a simple use case, and another for more advanced use. Let me make some changes and you could review then :)

The only thing I would change is the Configuration fields table that is not reflecting the format, I would prefer to have an entry for initial and one for end.

Noted, let me add to this.

Copy link

sonarcloud bot commented Oct 6, 2024

@Light2Dark Light2Dark marked this pull request as ready for review October 6, 2024 10:49
@thomaspoignant thomaspoignant merged commit 150d5f6 into thomaspoignant:main Oct 6, 2024
23 checks passed
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.

(doc) Improve progressive rollout documentation.
2 participants