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

docs: fix some vague description about analysis arguments #1672

Merged
merged 1 commit into from
Dec 14, 2021
Merged

docs: fix some vague description about analysis arguments #1672

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/features/analysis.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ templates together. The controller combines the `metrics` and `args` fields of a
The controller will error when merging the templates if:

* Multiple metrics in the templates have the same name
* Two arguments with the same name both have values
* Two arguments with the same name have different default values no matter the argument value in Rollout
Copy link
Member

Choose a reason for hiding this comment

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

I failed to understand this completely. Is this more clear by any chance?

"Two arguments with the same name have different default values. It doesn't matter what the argument values are in the Rollout."

Copy link
Member Author

Choose a reason for hiding this comment

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

The original text is wrong in that if Two arguments have different default values, the merging is valid. So the updated text adds the additional condition of have different default values

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Thanks for explaining :)


## Analysis Template Arguments

Expand All @@ -370,11 +370,11 @@ metadata:
name: args-example
spec:
args:
# required
# required in Rollout due to no default value
huikang marked this conversation as resolved.
Show resolved Hide resolved
- name: service-name
- name: stable-hash
- name: latest-hash
# optional
# optional in Rollout given the default value
huikang marked this conversation as resolved.
Show resolved Hide resolved
- name: api-url
value: http://example/measure
# from secret
Expand Down