. d Options doc wording#825
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the Options documentation to better explain how the shared Options object works with ApprovalTests verify functions and clarifies the description of its fluent interface, including referencing an example snippet. Sequence diagram for using Options with verify functionssequenceDiagram
actor User
participant Options as Options
participant ApprovalTests as verify_text
User->>Options: Options()
User->>Options: withFileExtension(ext)
Options-->>User: Options
User->>Options: withReporter(reporter)
Options-->>User: Options
User->>verify_text: verify_text(value, options)
verify_text-->>User: verification_result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There’s a typo in the fluent interface paragraph:
futhershould befurther. - Consider clarifying the phrase
verify...()(e.g.,verify*()or explicit examples) to avoid confusion about whether the ellipsis is literal or illustrative.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a typo in the fluent interface paragraph: `futher` should be `further`.
- Consider clarifying the phrase `verify...()` (e.g., `verify*()` or explicit examples) to avoid confusion about whether the ellipsis is literal or illustrative.
## Individual Comments
### Comment 1
<location path="approvaltests/docs/reference/Options.md" line_range="28" />
<code_context>
## Fluent Interface
-`Options` utilizes a fluent interface, allowing you to chain together commands. Each returned object is a new copy.
+`Options` has a "fluent" interface, allowing you to chain together commands: each returned object is a new copy of `Options` which you can configure futher. For example:
<!-- snippet: specify_all_the_options -->
</code_context>
<issue_to_address>
**issue (typo):** Typo: "futher" should be "further".
Please correct the misspelling to "further" in this sentence.
```suggestion
`Options` has a "fluent" interface, allowing you to chain together commands: each returned object is a new copy of `Options` which you can configure further. For example:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider splitting the new first sentence into two shorter sentences to improve readability (e.g., separate the idea of many verify functions from the idea of many optional parameters).
- Use consistent code formatting for function names (
verify...()vsverify()) and consider briefly explaining what the ellipsis means so readers aren’t confused about whether it’s literal syntax. - For the added example reference (
snippet: basic_approval_with_file_extension), add a brief lead-in sentence describing what the snippet demonstrates so readers understand why it’s relevant before seeing the code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider splitting the new first sentence into two shorter sentences to improve readability (e.g., separate the idea of many verify functions from the idea of many optional parameters).
- Use consistent code formatting for function names (`verify...()` vs `verify()`) and consider briefly explaining what the ellipsis means so readers aren’t confused about whether it’s literal syntax.
- For the added example reference (`snippet: basic_approval_with_file_extension`), add a brief lead-in sentence describing what the snippet demonstrates so readers understand why it’s relevant before seeing the code.
## Individual Comments
### Comment 1
<location path="approvaltests/docs/reference/Options.md" line_range="20" />
<code_context>
-There are many things you might want to tweak with Approval Tests. `Options` is the entry-point for many of the changes.
-It is on all `verify()` methods, as an optional parameter.
+ApprovalTests has many `verify...()` functions that need the same optional parameters, of which there are many. Instead of overloading every `verify...()` function in the same way, over and over, we put those options into an `Options` object that all the `verify...()` functions can take as an optional parameter.
+
+For example:
</code_context>
<issue_to_address>
**suggestion (typo):** Consider tightening the wording to avoid repetition and improve clarity.
The clause "that need the same optional parameters, of which there are many" is repetitive and a bit awkward. Consider something like "that share a large set of optional parameters" to keep the meaning while tightening the sentence.
```suggestion
ApprovalTests has many `verify...()` functions that share a large set of optional parameters. Instead of overloading every `verify...()` function in the same way, over and over, we put those options into an `Options` object that all the `verify...()` functions can take as an optional parameter.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| There are many things you might want to tweak with Approval Tests. `Options` is the entry-point for many of the changes. | ||
| It is on all `verify()` methods, as an optional parameter. | ||
| ApprovalTests has many `verify...()` functions that need the same optional parameters, of which there are many. Instead of overloading every `verify...()` function in the same way, over and over, we put those options into an `Options` object that all the `verify...()` functions can take as an optional parameter. |
There was a problem hiding this comment.
suggestion (typo): Consider tightening the wording to avoid repetition and improve clarity.
The clause "that need the same optional parameters, of which there are many" is repetitive and a bit awkward. Consider something like "that share a large set of optional parameters" to keep the meaning while tightening the sentence.
| ApprovalTests has many `verify...()` functions that need the same optional parameters, of which there are many. Instead of overloading every `verify...()` function in the same way, over and over, we put those options into an `Options` object that all the `verify...()` functions can take as an optional parameter. | |
| ApprovalTests has many `verify...()` functions that share a large set of optional parameters. Instead of overloading every `verify...()` function in the same way, over and over, we put those options into an `Options` object that all the `verify...()` functions can take as an optional parameter. |
Summary by Sourcery
Clarify and expand the documentation for the Options API in ApprovalTests, including introductory explanation and fluent interface description.
Documentation: