Add a kind to Issue.record to record various kinds a of issues #1120
Conversation
suzannaratcliff
requested review from
stmontgomery,
grynspan and
briancroom
as code owners
| @@ -86,10 +87,11 @@ extension Issue { | |||
| @discardableResult public static func record( | |||
| _ comment: Comment? = nil, | |||
| severity: Severity, | |||
| sourceLocation: SourceLocation = #_sourceLocation | |||
| sourceLocation: SourceLocation = #_sourceLocation, | |||
| kind: Kind = .unconditional | |||
There was a problem hiding this comment.
I recall this is the overload of record() you added previously, and its primary difference from the existing, public API is that it includes a severity: parameter. This one is currently @_spi(Experimental), and I think that SPI level makes sense given your active pitch which seeks to elevate this overload to non-SPI and replace the older one entirely. If that proposal advances and is accepted, I think we will want to expose this record() method as it's currently written and proposed, without including a kind: parameter in its signature.
Given those plans, I think it may make more sense in this PR to expose an entirely new overload of the record() function which includes the kind: parameter, and leave the existing overload alone. And that overload can have two SPI groups—@_spi(Experimental) and @_spi(ForToolsIntegrationOnly)—to reflect that it's intended for use by integrated tools.
I would also suggest placing kind: as the first parameter in that new overload, and giving its comment parameter a label (instead of _ comment: Comment?).
There was a problem hiding this comment.
It should never be possible to create an issue with an arbitrary kind field via API or non-experimental SPI, so we need to nix that argument for any public interfaces.
stmontgomery
added
enhancement
|
@swift-ci please test |
| @_spi(Experimental) | ||
| @_spi(ForToolsIntegrationOnly) | ||
| @discardableResult public static func record( | ||
| kind: Kind = .unconditional, |
There was a problem hiding this comment.
Ok now that this is a distinct overload, I think you'll need to make kind: not have a default value at all, because all parameters have a default value now and Issue.record() (zero args) may be ambiguous with the public API
| kind: Kind = .unconditional, | |
| kind: Kind, |
| /// - severity: The severity of the issue. | ||
| /// - sourceLocation: The source location to which the issue should be | ||
| /// attributed. | ||
| /// - kind: The kind of the issue. |
There was a problem hiding this comment.
kind can move up to the top, now
| @@ -92,6 +92,34 @@ extension Issue { | |||
| let issue = Issue(kind: .unconditional, severity: severity, comments: Array(comment), sourceContext: sourceContext) | |||
| return issue.record() | |||
| } | |||
|
|
|||
| /// Record an issue when a running test fails unexpectedly. | |||
There was a problem hiding this comment.
| /// Record an issue when a running test fails unexpectedly. | |
| /// Record an issue with a specific kind. |
Add a kind to Issue.record to record various kinds a of issues
Motivation:
I would like to be able to specify what kind of issue I am recording when I record an issue.
Modifications:
Add an optional parameter to
Issue.recordthat takes in an issueKind.Checklist: