-
Notifications
You must be signed in to change notification settings - Fork 18
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
Update examples so it is easier to test #56
Conversation
Reviewer's Guide by SourceryThis pull request updates several example values in the API model properties across multiple files. The changes include updating container image versions, command examples, stdout paths, and URLs to placeholders. These updates aim to make the examples more relevant and easier to test. File-Level Changes
Tips
|
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.
Hey @lvarin - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
@@ -142,7 +142,7 @@ public TesExecutor stdout(String stdout) { | |||
* Path inside the container to a file where the executor's stdout will be written to. Must be an absolute path. Example: ``` { \"stdout\" : \"/tmp/stdout.log\" } ``` | |||
* @return stdout | |||
*/ | |||
@ApiModelProperty(example = "/tmp/stdout.log", value = "Path inside the container to a file where the executor's stdout will be written to. Must be an absolute path. Example: ``` { \"stdout\" : \"/tmp/stdout.log\" } ```") | |||
@ApiModelProperty(example = "/data/outfile", value = "Path inside the container to a file where the executor's stdout will be written to. Must be an absolute path. Example: ``` { \"stdout\" : \"/tmp/stdout.log\" } ```") |
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.
suggestion: Update example path in the value description.
The example path in the example
attribute has been updated to /data/outfile
, but the value description still shows /tmp/stdout.log
. Consider updating the value description to match the new example path for consistency.
@ApiModelProperty(example = "/data/outfile", value = "Path inside the container to a file where the executor's stdout will be written to. Must be an absolute path. Example: ``` { \"stdout\" : \"/tmp/stdout.log\" } ```") | |
@ApiModelProperty(example = "/data/outfile", value = "Path inside the container to a file where the executor's stdout will be written to. Must be an absolute path. Example: ``` { \"stdout\" : \"/data/outfile\" } ```") |
@@ -190,7 +190,7 @@ public TesTask addOutputsItem(TesOutput outputsItem) { | |||
* Output files. Outputs will be uploaded from the executor container to long-term storage. | |||
* @return outputs | |||
*/ | |||
@ApiModelProperty(example = "[{\"path\":\"/data/outfile\",\"url\":\"s3://my-object-store/outfile-1\",\"type\":\"FILE\"}]", value = "Output files. Outputs will be uploaded from the executor container to long-term storage.") | |||
@ApiModelProperty(example = "[{\"path\":\"/data/outfile\",\"url\":\"s3://<CHANGE_THIS>/outfile-1\",\"type\":\"FILE\"}]", value = "Output files. Outputs will be uploaded from the executor container to long-term 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.
question: Placeholder in example URL.
The example URL contains a placeholder <CHANGE_THIS>
. Ensure that this is intentional and consider providing a more concrete example if possible.
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.
LGTM
Summary by Sourcery
Updated example values in API model properties to reflect more current or generic data, making it easier to test and understand the usage of the API.