Add highlights summary to bug report attachment #124
Conversation
| @@ -103,6 +103,6 @@ typedef void (^ARKEmailBugReporterCustomPromptCompletionBlock)(ARKEmailBugReport | |||
| @property (nonatomic) BOOL attachesViewHierarchyDescription; | |||
|
|
|||
| /// Returns an attachment containing the log messages. Defaults to a plain text attachment containing each log message formatted using the bug reporter's `logFormatter`. | |||
| - (nullable ARKBugReportAttachment *)attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName; | |||
| - (nullable ARKBugReportAttachment *)attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName numberOfErrorsInHighlights:(NSInteger)numberOfErrorsInHighlights; | |||
There was a problem hiding this comment.
This is a breaking change for AardvarkMailUI. We could potentially work around this, but I think we should go ahead and call this a breaking change, since there will be other breaking changes to follow (in making the log store be treated like any other attachment in the email bug reporter).
| @@ -107,11 +110,13 @@ public final class LogStoreAttachmentGenerator: NSObject { | |||
| /// - parameter logMessages: The log messages to be included in the attachment. | |||
| /// - parameter logFormatter: The formatter with which to format the log messages. | |||
| /// - parameter logStoreName: The name of the log store from which the logs were collected. | |||
| @objc(attachmentForLogMessages:usingLogFormatter:logStoreName:) | |||
| /// - parameter numberOfErrorsInHighlights: The number of errors to include in the highlights for the log store. | |||
| @objc(attachmentForLogMessages:usingLogFormatter:logStoreName:numberOfErrorsInHighlights:) | |||
There was a problem hiding this comment.
This is a breaking change for Aardvark. This one would be fairly easy to work around (by creating a separate method, rather than adding a parameter with a default value). I could go either way on this one.
There was a problem hiding this comment.
2¢ For such a small change, I think the workaround is preferable.
NickEntin
force-pushed
the
entin/highlights-summary
branch
4 times, most recently
from
19faefe
to
81c7b23
Compare
NickEntin
force-pushed
the
entin/highlights-summary
branch
from
81c7b23
to
6f50c48
Compare
Sources/Aardvark/BugReporting/LogStoreAttachmentGenerator.swift
Outdated
Show resolved
Hide resolved
Sources/Aardvark/BugReporting/LogStoreAttachmentGenerator.swift
Outdated
Show resolved
Hide resolved
NickEntin
force-pushed
the
entin/highlights-summary
branch
2 times, most recently
from
cb141a7
to
67020d5
Compare
NickEntin
force-pushed
the
entin/highlights-summary
branch
from
67020d5
to
ea60180
Compare
| @@ -124,10 +129,19 @@ public final class LogStoreAttachmentGenerator: NSObject { | |||
|
|
|||
| let fileName = logsFileName(for: logStoreName, fileType: "txt") | |||
|
|
|||
| let recentErrorSummary = logMessages | |||
| .lazy | |||
There was a problem hiding this comment.
Um, I'll be lazy: is .lazy doing anything here, given the isEmpty call just below? (I don't know!)
There was a problem hiding this comment.
I think the lazy still reduces work cause it at least keeps it from doing the map past the prefix. I'm not 100% sure though, I might be overestimating the power of lazy. At the very least it doesn't seem to hurt anything.
Resolves #119