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.

Sign up for GitHub

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

Jump to bottom

docs: Add configuration docs for apisix and env variables #7262

Closed
wants to merge 1 commit into from
Closed

docs: Add configuration docs for apisix and env variables #7262

wants to merge 1 commit into from

Conversation

avinal
Copy link
Contributor

@avinal avinal commented Jun 16, 2022

Description

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

Signed-off-by: Avinal Kumar [email protected]

@avinal
Copy link
Contributor Author

avinal commented Jun 16, 2022

I have added all the options under apisix in the default config YAML. Please let me know if something is missing.
Please review: @juzhiyuan @navendu-pottekkat @tzssangglass

@avinal avinal force-pushed the avinal/issue6611 branch 2 times, most recently from 4723865 to 537831c Compare June 16, 2022 08:27
@juzhiyuan
Copy link
Member

image

👋 Hi, please also add the missing category :)

guitu168
guitu168 reviewed Jun 16, 2022
View reviewed changes
docs/en/latest/configuration/config-yaml.md Outdated Show resolved Hide resolved
docs/en/latest/configuration/config-yaml.md Outdated Show resolved Hide resolved
docs/en/latest/configuration/configuration.md Outdated Show resolved Hide resolved
docs/en/latest/configuration/configuration.md Outdated Show resolved Hide resolved
@spacewander
Copy link
Member

20220624-145532

The new doc is quite confusing...

For example, it is very hard to figure out that the boolean is applied to the stream_proxy.only.

Can we add the comment in a reference of yaml, like https://github.com/elastic/beats/blob/master/filebeat/filebeat.reference.yml?

@avinal
Copy link
Contributor Author

avinal commented Jun 24, 2022

@spacewander are you suggesting that the YAML file should be updated?
FYI I am working on how can I make them more obvious so that people don't get confused with other options.

@spacewander
Copy link
Member

@spacewander are you suggesting that the YAML file should be updated? FYI I am working on how can I make them more obvious so that people don't get confused with other options.

I would suggest using a yaml code block in the doc. The yaml code will be a detailed commented version of config-default.yaml like the https://github.com/elastic/beats/blob/master/filebeat/filebeat.reference.yml

There are the benefits:

  1. straightforward
  2. easy to sync
  3. consistent render behavior

As APISIX is an open-source project, it would be great to reduce the requirement to update the doc so that contributors can update the doc themselves. Otherwise, the maintenance burden would be too heavy.

Also, a 100% mirrored config-default.yaml is easier for the users to refer to.

@avinal
Copy link
Contributor Author

avinal commented Jul 4, 2022

I am looking into alternative methods that will be easy to maintain as well as easy to read. May take some time. Thanks

@avinal avinal force-pushed the avinal/issue6611 branch 2 times, most recently from dba2aba to aacd128 Compare July 10, 2022 13:38
@avinal avinal force-pushed the avinal/issue6611 branch 4 times, most recently from 35295f3 to ef8f10f Compare July 10, 2022 13:49
@github-actions
Copy link

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 11, 2022
@avinal
docs: Add configuration docs for apisix and env variables
64262c1 
- Added Configuration chapter
- Added environment variables docs
- Added docs for `apisix` in conf/config-default.yaml
- Partially solves apache#6611

Signed-off-by: Avinal Kumar <[email protected]>
@avinal
Copy link
Contributor Author

avinal commented Sep 24, 2022

I apologize for the long delay, Please review. I have addressed all the comments.

@github-actions github-actions bot removed the stale label Sep 25, 2022
spacewander
spacewander reviewed Sep 25, 2022
View reviewed changes
docs/en/latest/configuration/config-default.md
admin_ssl_ca_cert: ""
```

### `admin_listen`
Copy link
Member

Choose a reason for hiding this comment

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

Could you update this PR according to the latest https://github.com/apache/apisix/blob/master/conf/config-default.yaml?

We have just made a big refactor for 3.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure let me update

@github-actions
Copy link

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 25, 2022
@github-actions
Copy link

This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juzhiyuan juzhiyuan juzhiyuan left review comments

@spacewander spacewander spacewander left review comments

@tzssangglass tzssangglass tzssangglass left review comments

@SkyeYoung SkyeYoung SkyeYoung left review comments

@guitu168 guitu168 Awaiting requested review from guitu168

Assignees
No one assigned
Labels
stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@avinal @juzhiyuan @spacewander @tzssangglass @SkyeYoung @guitu168