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

Set minimum iOS support to iOS 11 #139

Merged
merged 1 commit into from Mar 8, 2023
Merged

Conversation

antranapp
Copy link
Contributor

@antranapp antranapp commented Dec 17, 2022

We would like to integrate this great package into our app but our app is still supporting iOS 11+.

It looks like that the main functionality of the package is available for iOS 11, only a default SmartInsight requires iOS 13+ due to Combine

I have added some availability checks for that SmartInsight and set the minimum iOS platform version to 11.

Copy link

@BasThomas BasThomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I'm not sure how others feel about taking care of support for iOS 11+, though... 😄

Sources/Reporters/SmartInsightsReporter.swift Outdated Show resolved Hide resolved
Sources/SmartInsights/UpdateAvailableInsight.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@AvdLee AvdLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine adding support for iOS 11 for now. As soon as we run into issues later on, we can always drop support again. Looking at the changes, however, it seems low effort now 👌

@github-actions
Copy link

github-actions bot commented Feb 6, 2023

This PR is stale because it has been open for 30 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

Copy link

@BasThomas BasThomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Thanks again @antranapp :)

@BasThomas
Copy link

Oh, one more thing. We've been requiring signed commits for a while. Can you rebase and sign your commits by chance, @antranapp? Let me know if that is something you'd need assistance with.

Signed-off-by: An Tran <tran.binhan@gmail.com>
@antranapp
Copy link
Contributor Author

@BasThomas I have signed my commit and force push again.

@wetransferplatform
Copy link
Collaborator

Messages
📖 DiagnosticsTests: Executed 33 tests (0 failed, 0 retried, 0 skipped) in 10.779 seconds
📖

View more details on Bitrise

Code Coverage Report

Name Coverage
Diagnostics 70.37% ⚠️

SwiftLint found issues

Severity File Reason
Warning MFMailComposeViewControllerTests.swift:10 Imports should be sorted. (sorted_imports)
Warning MFMailComposeViewControllerTests.swift:26 Lines should not have trailing whitespace. (trailing_whitespace)
Warning MFMailComposeViewControllerTests.swift:35 A 'type_method' should not be placed amongst the type content(s) 'initializer,instance_property'. (type_contents_order)
Warning SmartInsightsReporter.swift:56 Prefer reduce(into:_:) over reduce(_:_:) for copy-on-write types (reduce_into)
Warning UpdateAvailableInsightTests.swift:41 Line should be 140 characters or less: currently 157 characters (line_length)
Warning UpdateAvailableInsightTests.swift:54 Line should be 140 characters or less: currently 157 characters (line_length)
Warning UpdateAvailableInsightTests.swift:67 Line should be 140 characters or less: currently 157 characters (line_length)
Warning UpdateAvailableInsightTests.swift:10 Imports should be sorted. (sorted_imports)
Warning UpdateAvailableInsightTests.swift:11 Imports should be sorted. (sorted_imports)
Warning UpdateAvailableInsight.swift:24 Use shorthand syntax for optional binding (shorthand_optional_binding)
Warning UpdateAvailableInsight.swift:32 Use shorthand syntax for optional binding (shorthand_optional_binding)
Warning UpdateAvailableInsight.swift:43 Use shorthand syntax for optional binding (shorthand_optional_binding)
Warning UpdateAvailableInsight.swift:59 Use shorthand syntax for optional binding (shorthand_optional_binding)
Warning UpdateAvailableInsight.swift:48 Lines should not have trailing whitespace. (trailing_whitespace)

Generated by 🚫 Danger Swift against 09f542d

@AvdLee AvdLee merged commit 75d2625 into WeTransfer:master Mar 8, 2023
@AvdLee
Copy link
Contributor

AvdLee commented Mar 8, 2023

@antranapp thanks for your contribution!

@BasThomas
Copy link

Awesome, thanks again @antranapp and @AvdLee for merging! :)

@wetransferplatform
Copy link
Collaborator

Congratulations! 🎉 This was released as part of Release 4.4.0 🚀

Generated by GitBuddy

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.

None yet

5 participants