Swiftify API #504
Merged
Swiftify API #504
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bamx23
reviewed
Jun 22, 2024
| * used to store the output of its respective filter in the final | ||
| * report dictionary. | ||
| */ | ||
| + (instancetype) filterWithFilters:(NSArray*) filters keys:(NSArray<NSString*>*) keys; |
There was a problem hiding this comment.
Mind providing a type to array?
There was a problem hiding this comment.
Can't. Here is a bad legacy design, where we can pass array of objects or array of arrays. From doc:
* @param filters An array of filters to apply. Each filter should conform to the
* KSCrashReportFilter protocol. If a filter is an NSArray, it will
* be wrapped in a pipeline filter.
| @@ -56,15 +61,37 @@ | |||
| * Each "filter" can be id<KSCrashReportFilter> or an NSArray | |||
| * of filters (which gets wrapped in a pipeline filter). | |||
| */ | |||
| + (KSCrashReportFilterCombine*) filterWithFiltersAndKeys:(id) firstFilter, ... NS_REQUIRES_NIL_TERMINATION; | |||
| + (instancetype) filterWithFiltersAndKeys:(nullable id) firstFilter, ... NS_REQUIRES_NIL_TERMINATION; | |||
There was a problem hiding this comment.
Should we drop nil-termenated initialization? Not sure if it's used anywhere, but maybe it's just me
There was a problem hiding this comment.
It's visible for C, but not for Swift, that's why a implemented with NSArray side-by-side.
Comment on lines
+239
to
+241
| KSCRF_DEFINE_CONSTANT(KSCrashField, BuildType, buildType, "build_type") | ||
|
|
||
| KSCRF_DEFINE_CONSTANT(KSCrashField, MemoryFootprint, memoryFootprint, "memory_footprint") |
There was a problem hiding this comment.
Should we also split KSCrashField into smaller groups?
There was a problem hiding this comment.
I was thinking about it, but I didn't want to break the names.
…essary copy Co-authored-by: Nikolay Volosatov <code@bamx23.com>
Co-authored-by: Nikolay Volosatov <code@bamx23.com>
…ppropriate types Co-authored-by: Nikolay Volosatov <code@bamx23.com>
bamx23
approved these changes
Jun 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces significant updates to the KSCrash library, focusing on enhancing compatibility with Swift, modernizing the Objective-C code, and improving maintainability. Below are the key changes made:
Major Changes:
instancetypeto ensure better compatibility and consistency across Swift and Objective-C. This change improves type safety and makes the codebase more modern and reliable.nullableannotations to parameters and return types, where applicable, to clarify which values can benil. This change is crucial for Swift interoperability, reducing potential runtime errors and improving code clarity.NS_ENUM:NS_ENUMto enhance type safety and facilitate smoother integration with Swift. This modification allows enums to be used more effectively in Swift, leveraging the language's type system for better safety and usability.@KSCrashField_XXXstring literal access withKSCrashField_XXXconstants to align with contemporary Objective-C practices. This change reduces the risk of errors associated with string literals and ensures a more maintainable codebase.NS_SWIFT_NAMEannotations to key methods and properties to provide more intuitive names when accessed from Swift. This improves the developer experience, making the API more natural and easier to use in Swift projects.