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

Cleaning recently changed docs #924

Merged
merged 13 commits into from
Feb 13, 2025

Conversation

iamleeg
Copy link
Contributor

@iamleeg iamleeg commented Jan 24, 2025

Improving the style and explanation of recent additions to the documentation.

Motivation:

The recent changes to test traits are great, and come with some very nice documentation. I've done what I can to improve the style and the wording to make it easier to understand, and to make the style consistent with other documentation in the project.

Modifications:

Changed the documentation for Trait and associated types.

Result:

There are no behavioral changes in this PR, but I hope that the existing behavior is easier to use with better documentation.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

@stmontgomery stmontgomery added documentation Improvements or additions to documentation public-api Affects public API labels Jan 24, 2025
@stmontgomery stmontgomery added this to the Swift 6.x milestone Jan 24, 2025
Comment on lines 18 to 22
/// Use this type to specify a test timeout with ``TimeLimitTrait``.
/// Use this type instead of Swift's built-in `Duration`
/// type because the testing library doesn't support high-precision,
/// arbitrarily short durations for test timeouts.
/// The smallest allowed unit of time is minutes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Purely stylistic, but it seems like we could re-flow the sentences here to condense these lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I re-worded this to avoid saying "Use this type" twice and it ended up just as long, but see what you think.

Comment on lines 73 to 75
/// The testing library can use a shorter time limit than that specified by
/// `timeLimit` may be reduced if it's configured to enforce a maximum
/// per-test limit. When you configure a maximum per-test limit, the time
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the first sentence of this paragraph was only partly reworded and needs another look

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK had another go at this paragraph, see what you think.

Comment on lines 53 to 56
/// Get this trait's scope provider for the specified test and/or test case,
/// if any.
/// Get this trait's scope provider for the specified test or test case.
Copy link
Contributor

Choose a reason for hiding this comment

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

The "and/or" was there because, there is always a Test passed, it's just the Test.Case parameter which is optional. Any ideas for a better way to phrase that? Saying "test or test case" makes it sound like it's one or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good point. I avoided and/or because it's difficult for readers of English as a second language. I've gone with "test, and optional test case". I'm not sure I'm entirely happy with that but maybe it helps advance the discussion :).

@iamleeg iamleeg requested a review from grynspan January 29, 2025 13:56
@iamleeg iamleeg requested a review from grynspan February 7, 2025 16:55
/// This means that by default, a suite trait will _either_ provide its
/// custom scope once for the entire suite, or once per-test function it
/// contains.
/// is `true`, then this method returns `nil`, and the suite trait
Copy link
Contributor

Choose a reason for hiding this comment

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

This discussion only applies to suite traits (it's saying "if the suite is recursive, then this function is recursively called for tests in the suite.")

Copy link
Contributor

Choose a reason for hiding this comment

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

@stmontgomery I find this whole discussion somewhat confusing. Feels like we need a flowchart.

@iamleeg iamleeg requested a review from grynspan February 11, 2025 10:31
@iamleeg iamleeg merged commit 44684c1 into main Feb 13, 2025
@iamleeg iamleeg deleted the iamleeg/143477064-cleaning-recently-changed-docs branch February 13, 2025 13:15
iamleeg added a commit that referenced this pull request Feb 13, 2025
Improving the style and explanation of recent additions to the
documentation.

The recent changes to test traits are great, and come with some very
nice documentation. I've done what I can to improve the style and the
wording to make it easier to understand, and to make the style
consistent with other documentation in the project.

Changed the documentation for `Trait` and associated types.

There are no behavioral changes in this PR, but I hope that the existing
behavior is easier to use with better documentation.

- [X] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [X] If public symbols are renamed or modified, DocC references should
be updated.
iamleeg added a commit that referenced this pull request Feb 13, 2025
Improving the style and explanation of recent additions to the
documentation.

The recent changes to test traits are great, and come with some very
nice documentation. I've done what I can to improve the style and the
wording to make it easier to understand, and to make the style
consistent with other documentation in the project.

Changed the documentation for `Trait` and associated types.

There are no behavioral changes in this PR, but I hope that the existing
behavior is easier to use with better documentation.

- [X] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [X] If public symbols are renamed or modified, DocC references should
be updated.
iamleeg added a commit that referenced this pull request Feb 13, 2025
Improving the style and explanation of recent additions to the
documentation.

The recent changes to test traits are great, and come with some very
nice documentation. I've done what I can to improve the style and the
wording to make it easier to understand, and to make the style
consistent with other documentation in the project.

Changed the documentation for `Trait` and associated types.

There are no behavioral changes in this PR, but I hope that the existing
behavior is easier to use with better documentation.

- [X] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [X] If public symbols are renamed or modified, DocC references should
be updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation public-api Affects public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants