Rename note to notification throughout the codebase #893
Conversation
ahoppen
left a comment
ahoppen
left a comment
There was a problem hiding this comment.
Thanks @issamarabi. I have a few comments inline but otherwise it looks good to me.
| /// If this is from a note, the note's description should be passed as `fromNote`. | ||
| init?(fixits: SKDResponseArray, in snapshot: DocumentSnapshot, fromNote: String?) { | ||
| /// If this is from a notification, the notification's description should be passed as `fromNotification`. | ||
| init?(fixits: SKDResponseArray, in snapshot: DocumentSnapshot, fromNotification: String?) { |
There was a problem hiding this comment.
This file is about diagnostics, which do indeed have notes, not notifications. Could you change it back?
There was a problem hiding this comment.
I changed these back. Let me know if this works!
| var newDiags: [CachedDiagnostic] = [] | ||
| sourcekitdDiagnostics?.forEach { _, diag in | ||
| if let diag = CachedDiagnostic(diag, in: snapshot, useEducationalNoteAsCode: supportsCodeDescription) { | ||
| if let diag = CachedDiagnostic(diag, in: snapshot, useEducationalNotificationAsCode: supportsCodeDescription) { |
There was a problem hiding this comment.
This is about notes, not notifications, so useEducationalNoteAsCode is correct.
| } | ||
| } else { | ||
| XCTFail("missing '?' note") | ||
| XCTFail("missing '?' notification") |
There was a problem hiding this comment.
This is talking about diagnostics and should be note.
| } | ||
| } else { | ||
| XCTFail("missing '!' note") | ||
| XCTFail("missing '!' notification") |
There was a problem hiding this comment.
Same here, talking about diagnostics.
Co-authored-by: Alex Hoppen <[email protected]>
Co-authored-by: Alex Hoppen <[email protected]>
Co-authored-by: Alex Hoppen <[email protected]>
Co-authored-by: Alex Hoppen <[email protected]>
|
Argh, looks like this conflicted quite heavily with #896. Could you rebase the PR on top of Sorry for causing the conflicts. |
|
The issue has been resolved by #1353. |
2 participants
Fixes #865
notewithnotification.Originated from feedback by @ahoppen regarding terminology confusion. This change aims to maintain clarity across the codebase.
Reference: Apple’s issue tracker rdar://116703667
Please review.