-
Notifications
You must be signed in to change notification settings - Fork 189
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
add more config doc descriptions #3973
Conversation
Co-authored-by: Phil Davis <[email protected]>
"This branch is 4 commits ahead, 39 commits behind owncloud:master." Will that matter? Is it worth rebasing to make sure that it's all good? |
💥 Acceptance test Core-API-Tests-ocis-storage-10 failed. Further test are cancelled... |
GatewayAddress string `yaml:"gateway_addr" env:"STORAGE_GATEWAY_GRPC_ADDR" desc:"GRPC address of the storage-system extension."` | ||
StorageAddress string `yaml:"storage_addr" env:"STORAGE_GRPC_ADDR" desc:"GRPC address of the storage-system extension."` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These have the same description - are there 2 variables that do the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's a somehow weird decision to expose both settings. I think this grew organically and has historical reasons.
Is now tracked in #4058
@wkloucek see the image below, it seems that in the web service, one config var errors. It is between |
|
||
This service provides search functionality. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is obvious that a search service provides search functionality, but could we have a bit more meat on the bone 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let us do abstracts in subsequent PRs. They should be more than one sentence.
CommitShareToStorageGrant bool `yaml:"commit_share_to_storage_grant" env:"GATEWAY_COMMIT_SHARE_TO_STORAGE_GRANT" desc:"Commit shares to storage grants."` | ||
CommitShareToStorageRef bool `yaml:"commit_share_to_storage_ref" env:"GATEWAY_COMMIT_SHARE_TO_STORAGE_REF" desc:"Commit shares to storage."` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference in between the two? Or what is the impact is setting true/false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GATEWAY_COMMIT_SHARE_TO_STORAGE_REF
was removed and the description updated in 4fe5703
services/graph/pkg/config/http.go
Outdated
Namespace string `yaml:"-"` | ||
Root string `yaml:"root" env:"GRAPH_HTTP_ROOT"` | ||
Root string `yaml:"root" env:"GRAPH_HTTP_ROOT" desc:"The root path of the HTTP service."` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default is: /graph
. In which folder can /graph be found?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarified with @dragonchaser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to Subdirectory that serves as the root for this HTTP service.
for all services that have that option.
if you have oCIS on https://ocis.owncloud.test
, the graph service is available on https://ocis.owncloud.test/graph
. If you want to have it on another subdirectory, eg. /foobar
, you could change that config and also the proxy route to serve the graph on https://ocis.owncloud.test/foobar
Co-authored-by: Martin <[email protected]> Co-authored-by: Phil Davis <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, lets merge this and start a new PR to fix open/remaining/improvable stuff.
(But it would be great if you have the time to look at the one open env-question above before merging so it will not get "lost"...)
Already fixed in this PR |
@phil-davis 👍 😄 |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
👍 |
Description
adds more config doc descriptions, removes unused configuration options
Related Issue
Motivation and Context
have all config options described
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: