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: Migrate /docs items to website #825

Closed
wants to merge 26 commits into from
Closed

docs: Migrate /docs items to website #825

wants to merge 26 commits into from

Conversation

agardnerIT
Copy link
Contributor

@agardnerIT agardnerIT commented Aug 9, 2023

This PR

  • Creates reference menu item on website
  • Migrates docs/configuration/configuration.md to website
  • Migrates docs/configuration/flagd.md to website
  • Migrates docs/configuration/flagd_start.md to website

Related Issues

Follow-up Tasks

How to test

@agardnerIT agardnerIT requested a review from a team as a code owner August 9, 2023 06:15
@netlify
Copy link

netlify bot commented Aug 9, 2023

Deploy Preview for polite-licorice-3db33c ready!

Name Link
🔨 Latest commit 252b382
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/65015a58569c5100086febfb
😎 Deploy Preview https://deploy-preview-825--polite-licorice-3db33c.netlify.app/reference/flag_configuration
📱 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.

@agardnerIT agardnerIT changed the title docs: add menu items and pages. docs: Migrate /docs items to website Aug 9, 2023
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
This reverts commit cd56c7e.

Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
@agardnerIT
Copy link
Contributor Author

this is good to go. Don't know why the diff is failing

@agardnerIT
Copy link
Contributor Author

Some lessons learned here:

  1. make generate-docs fails on windows:
make generate-docs
FIND: Parameter format not correct
cd flagd; go run ./cmd/doc/main.go
The system cannot find the path specified.
make: *** [Makefile:74: generate-docs] Error 1
  1. Changing dir into /flagd then running the go command with go < 1.20.6 failed with an Exception in runtime.cgocall.
    I needed to update to golang 1.21.0 and it worked

  2. The commands alter my files in a way that I do not want. I don't want that boilerplate as we're moving away from /docs so I do want a single line pointing people to the website.

git diff ../docs/configuration/flagd.md
diff --git a/docs/configuration/flagd.md b/docs/configuration/flagd.md
index bcfa899..97c4ae6 100644
--- a/docs/configuration/flagd.md
+++ b/docs/configuration/flagd.md
@@ -1,3 +1,18 @@
-# flagd
+<!-- markdownlint-disable-file -->
+## flagd
+
+Flagd is a simple command line tool for fetching and presenting feature flags to services. It is designed to conform to Open Feature schema for flag definitions.
+
+### Options
+
+```
+      --config string   config file (default is $HOME/.agent.yaml)
+  -x, --debug           verbose logging
+  -h, --help            help for flagd
+```
+
+### SEE ALSO
+
+* [flagd start](flagd_start)    - Start flagd
+* [flagd version](flagd_version)        - Print the version number of FlagD

-[https://flagd.dev](https://flagd.dev) is the new home of flagd documentation.

Long story short, I think we need to revisit the conditions applied in the PR workflow that enforce these checks for the new website structure.

Second, whatever is decided to be necessary could probably just be executed on the Action runner to remove this step from the user.

@agardnerIT
Copy link
Contributor Author

@Kavindu-Dodan do you have any ideas on how to resolve the above to get this merged? Thanks!

Signed-off-by: Kavindu Dodanduwa <[email protected]>
@Kavindu-Dodan
Copy link
Contributor

Kavindu-Dodan commented Aug 25, 2023

@Kavindu-Dodan do you have any ideas on how to resolve the above to get this merged? Thanks!

I resolved the conflicts. But not sure how to fix the genere docs 🤔

we could disable the checks from build as we have a new website for docs

@beeme1mr & @toddbaert any opinion on this ?

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Merging #825 (252b382) into main (5a41226) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #825   +/-   ##
=======================================
  Coverage   72.73%   72.73%           
=======================================
  Files          28       28           
  Lines        2857     2857           
=======================================
  Hits         2078     2078           
  Misses        683      683           
  Partials       96       96           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@beeme1mr
Copy link
Member

@Kavindu-Dodan can't we just change the location of the generated doc?

https://github.com/open-feature/flagd/blob/main/flagd/cmd/doc/main.go#L9

Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
Signed-off-by: Adam Gardner <[email protected]>
@agardnerIT
Copy link
Contributor Author

This should now be all good and ready for a re-review (apart from the make generate-docs) failure above.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this file should be moved to the website. A CONTRIBUTING file has special meeting in GitHub and anyone contributing would expect the content to live there.

Comment on lines 214 to 215
- [starts_with](https://github.com/open-feature/flagd/blob/main/docs/configuration/string_comparison_evaluation.md#startswith-evaluation-configuration)
- [ends_with](https://github.com/open-feature/flagd/blob/main/docs/configuration/string_comparison_evaluation.md#endswith-evaluation-configuration)
Copy link
Member

Choose a reason for hiding this comment

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

Can these also be updated to a relative path?

Comment on lines +262 to +263
"fractionalEvaluation": [
"email",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"fractionalEvaluation": [
"email",
"fractional": [
{ "var": "email" },

@beeme1mr
Copy link
Member

Duplicate of #927

@beeme1mr beeme1mr marked this as a duplicate of #927 Sep 27, 2023
@beeme1mr beeme1mr closed this Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment