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

Feature/modify base manifest sample #5

Merged
merged 3 commits into from
Jan 15, 2024

Conversation

okmtz
Copy link
Collaborator

@okmtz okmtz commented Jan 15, 2024

Description

  • modify sample config/base_manifest.yaml parallelism field value.
  • add explanation about type matching between config/base_manifest.yaml and Gatling struct field in related docs.

Checklist

Please check if applicable

  • Tests have been added (if applicable, ie. when cli codes are added or modified)
  • Relevant docs have been added or modified (if applicable, ie. when new features are added or current features are modified)

@okmtz okmtz requested a review from a team January 15, 2024 02:43
Therefore, setting values to fields marked `<config.yaml overrides this field>` in `base_manifest.yaml` is not necessary.
Fields marked `<config.yaml overrides this field>` in `base_manifest.yaml` are set to different values for each load test. The value of this field will be replaced by the value in `config.yaml` respectively when Gatling Commander runs. Therefore, setting values to fields marked `<config.yaml overrides this field>` in `base_manifest.yaml` is not necessary.

\* Gatling Commander once loads `base_manifest.yaml` value to Gatling struct object before replaced by `config.yaml`. So the type of `base_manifest.yaml` field vlaue must be matched to Gatling struct field one. If type not match, like following error will be occured.
Copy link
Contributor

Choose a reason for hiding this comment

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

@okmtz
【IMO】
Corrected sentences.

Suggested change
\* Gatling Commander once loads `base_manifest.yaml` value to Gatling struct object before replaced by `config.yaml`. So the type of `base_manifest.yaml` field vlaue must be matched to Gatling struct field one. If type not match, like following error will be occured.
\* Gatling Commander loads `base_manifest.yaml` value to Gatling struct object before it replaces the value by `config.yaml`. So the type of `base_manifest.yaml` field value must be matched to Gatling struct field one. If type not match, an error like following will occur.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kayamin
commit suggestion and rebase with my old commit !
thank you !

@okmtz okmtz force-pushed the feature/modify-base-manifest-sample branch from 7bce214 to fd0a81e Compare January 15, 2024 03:10
@okmtz okmtz requested a review from kayamin January 15, 2024 03:14
docs/user-guide.md Outdated Show resolved Hide resolved
@okmtz okmtz requested a review from kayamin January 15, 2024 03:18
Copy link
Contributor

@kayamin kayamin left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

@kenz kenz left a comment

Choose a reason for hiding this comment

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

一点だけコメントしました

`base_manifest.yaml``<config.yaml overrides this field>`と記載があるフィールドは、負荷試験ごとに異なる値が設定されます。
これらのフィールドの値は、Gatling Commanderの実行時に`config.yaml`の値でそれぞれ置き換えられます。
そのため、`base_manifest.yaml`での値の変更は不要です。
`base_manifest.yaml``<config.yaml overrides this field>`と記載があるフィールドは、負荷試験ごとに異なる値が設定されます。これらのフィールドの値は、Gatling Commanderの実行時に`config.yaml`の値でそれぞれ置き換えられます。そのため、`base_manifest.yaml`での値の変更は不要です。
Copy link

Choose a reason for hiding this comment

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

【IMO】 base_manifest.yamlによる値の変更がいつの話か、どのようなメリットがあるかを明示するとよさそう。

Suggested change
`base_manifest.yaml``<config.yaml overrides this field>`と記載があるフィールドは、負荷試験ごとに異なる値が設定されます。これらのフィールドの値は、Gatling Commanderの実行時に`config.yaml`の値でそれぞれ置き換えられます。そのため、`base_manifest.yaml`での値の変更は不要です。
`base_manifest.yaml``<config.yaml overrides this field>`と記載があるフィールドは、負荷試験ごとに`config.yaml`の値で上書きされます。そのため、`base_manifest.yaml`の値を変更する事なく複数の負荷試験を連続して実行できます。```

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

別PRに分けて修正します!

@okmtz okmtz merged commit 0acb356 into main Jan 15, 2024
3 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.

3 participants