Skip to content
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

Fix swift lint errors #1348

Merged
merged 3 commits into from
Dec 15, 2023
Merged

Fix swift lint errors #1348

merged 3 commits into from
Dec 15, 2023

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Dec 8, 2023

Description

One Line Summary

Fix swiftlint errors so that our linting and CI can succeed.

Details

1. Ran swiftlint --fix commit

  • This remedied auto-correctable issues like trailing whitespace, unnecessary brackets

2. Organize User module files commit

  • Make folder for Executors and move executor files into this folder
  • Put OSUserExecutor class into its own file, removed from the OSUserRequests mega-file
  • Make folder for Requests and move request-related files to this folder
  • Split up each request from OSUserRequests mega-file into its own file
  • Now OSUserRequest.swift will contain just the protocol and an extension of OneSignalRequest

3. Organize OSUserExecutor functionality with a class extension commit

  • This is motivated by this swiftlint error about a class having too many lines in it:

"...OSUserExecutor.swift:36:1: error: Type Body Length Violation: Type body should span 350 lines or less excluding comments and whitespace: currently spans 401 lines (type_body_length)"

  • Move the parsing functionality of the OSUserExecutor into an extension of the class
  • The alternative considered would be to update the .swiftlint.yml file to override the type_body_length rule to accept a longer length.

Motivation

Part of making our CI succeed as well as actually utilizing our linter.

Scope

  • Linting
  • re-organizing files
  • no functionality change

Before         After

Testing

Unit testing

None

Manual testing

None, ensure SDK builds and app runs after changes.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

* Make folder for `Executors` and move executor files into this folder
* Put `OSUserExecutor` class into its own file, removed from the `OSUserRequests` mega-file
* Make folder for `Requests` and move request-related files to this folder
* Split up each request from `OSUserRequests` mega-file into its own file
* Now `OSUserRequest.swift` will contain just the protocol and an extension of OneSignalRequest
* Move the parsing functionality of the `OSUserExecutor` into an extension
* This is motivated by this swiftlint error: "...OSUserExecutor.swift:36:1: error: Type Body Length Violation: Type body should span 350 lines or less excluding comments and whitespace: currently spans 401 lines (type_body_length)"
* The alternative considered would be to update the `.swiftlint.yml` file to override the `type_body_length` rule to accept a longer length.
@nan-li nan-li force-pushed the add_get_onesignal_id branch from d9792fe to 06ab04a Compare December 11, 2023 19:59
Base automatically changed from add_get_onesignal_id to main December 15, 2023 20:23
@nan-li nan-li merged commit 4253e09 into main Dec 15, 2023
1 of 2 checks passed
@nan-li nan-li deleted the fix_swift_lint branch December 15, 2023 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants