-
Notifications
You must be signed in to change notification settings - Fork 128
Provide prose documentation for attachments. #1413
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
base: main
Are you sure you want to change the base?
Conversation
This PR adds prose documentation explaining how to create attachments and add them to tests. Resolves #1143.
| /// | ||
| /// - ``Trait/savingAttachments(if:)`` | ||
| /// | ||
| /// By default, the testing library saves your attachments as soon as you call |
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.
This file is all SPI.
|
Tracked internally as rdar://164478451. |
jerryjrchen
left a comment
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.
Looks great!
Sources/Testing/Attachments/Images/Attachment+AttachableAsImage.swift
Outdated
Show resolved
Hide resolved
jgmcnutt
left a comment
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.
Some minor suggestions. Please let me know if you have any questions.
Sources/Testing/Attachments/Images/Attachment+AttachableAsImage.swift
Outdated
Show resolved
Hide resolved
Sources/Testing/Attachments/Images/Attachment+AttachableAsImage.swift
Outdated
Show resolved
Hide resolved
Sources/Testing/Attachments/Images/Attachment+AttachableAsImage.swift
Outdated
Show resolved
Hide resolved
| ### Attach data or strings | ||
|
|
||
| If your test produces encoded data that you want to save as an attachment, you | ||
| can call ``Attachment/record(_:named:sourceLocation:)``: |
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.
| can call ``Attachment/record(_:named:sourceLocation:)``: | |
| can call ``Attachment/record(_:named:sourceLocation:)``. |
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.
Using a colon here to introduce a code sample/snippet is pretty standard practice. Compare this (previously-reviewed) article: https://developer.apple.com/documentation/testing/enablinganddisabling
Since we're already doing it in other files, I'd prefer to remain consistent, but could I ask you to get a DevPubs verdict and, if changes are needed, we can apply them globally in another PR?
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.
DevPubs verdict: We use the colon when we mention the snippet, such as "The following code example shows..." and "In the code below, X does Z....". If we don't mention the code, it isn't an introductory sentence and takes a period.
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.
Is it sufficient to end the sentence(s) I have now and then add "For example:"? I do want to make sure it's clear that the following code is related to the preceding text and not just appearing out of nowhere.
| If you can reliably estimate in advance how large the encoded representation | ||
| will be, implement ``Attachable/estimatedAttachmentByteCount``. The testing | ||
| library uses the value of this property as a hint to optimize memory and disk | ||
| usage: |
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.
| usage: | |
| usage. |
| You can also implement ``Attachable/preferredName(for:basedOn:)`` if you wish to | ||
| customize the name of the attachment when it is saved: |
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.
| You can also implement ``Attachable/preferredName(for:basedOn:)`` if you wish to | |
| customize the name of the attachment when it is saved: | |
| You can also implement ``Attachable/preferredName(for:basedOn:)`` if you want to | |
| customize the name of the attachment when saving it. |
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.
Holding for feedback on : vs .
…e.swift Co-authored-by: jgmcnutt <jgmcnutt@me.com>
…e.swift Co-authored-by: jgmcnutt <jgmcnutt@me.com>
…e.swift Co-authored-by: jgmcnutt <jgmcnutt@me.com>
Co-authored-by: jgmcnutt <jgmcnutt@me.com>
Co-authored-by: jgmcnutt <jgmcnutt@me.com>
Co-authored-by: jgmcnutt <jgmcnutt@me.com>
Co-authored-by: jgmcnutt <jgmcnutt@me.com>
Co-authored-by: jgmcnutt <jgmcnutt@me.com>
Co-authored-by: jgmcnutt <jgmcnutt@me.com>
Co-authored-by: jgmcnutt <jgmcnutt@me.com>
This PR adds prose documentation explaining how to create attachments and add them to tests.
Resolves #1143.
Checklist: