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

Update Gems and remove R.swift #8

Closed
wants to merge 5 commits into from

Conversation

bcylin
Copy link
Collaborator

@bcylin bcylin commented Aug 5, 2019

It seems to be very difficult to migrate the project from Swift 3 to Swift 5 at once. One possible approach is to reduce third party dependencies and update different parts of the project separately.

This PR updates the Gems and removes the dependency of R.swift, which caused a problem in some versions of Xcode and CocoaPods.

TBD:

  • Replace Freddy with Swift Codable
  • Replace Quick and Nimble with XCTest
  • Update the project to Xcode 10.1 with Swift 4.2
  • Update the project to Xcode 10.3 with Swift 5

@bcylin bcylin requested a review from dlackty August 5, 2019 23:03
let alert = UIAlertController(title: R.string.localizable.errorTitle(), message: message, preferredStyle: .alert)
alert.addAction(UIAlertAction(title: R.string.localizable.ok(), style: .default) { [weak self] _ in
let message = NSLocalizedString("video-error", comment: "") + "\n" + NSLocalizedString("contact-info", comment: "")
let alert = UIAlertController(title: NSLocalizedString("error-title", comment: ""), message: message, preferredStyle: .alert)

Choose a reason for hiding this comment

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

Line Length Violation: Line should be 120 characters or less: currently 131 characters (line_length)

let message = R.string.localizable.videoError() + "\n" + R.string.localizable.contactInfo()
let alert = UIAlertController(title: R.string.localizable.errorTitle(), message: message, preferredStyle: .alert)
alert.addAction(UIAlertAction(title: R.string.localizable.ok(), style: .default) { [weak self] _ in
let message = NSLocalizedString("video-error", comment: "") + "\n" + NSLocalizedString("contact-info", comment: "")

Choose a reason for hiding this comment

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

Line Length Violation: Line should be 120 characters or less: currently 121 characters (line_length)

@bcylin bcylin force-pushed the feature/remove-rswift branch 2 times, most recently from 6edb6ee to 01569e7 Compare August 5, 2019 23:50
@codecov
Copy link

codecov bot commented Aug 5, 2019

Codecov Report

Merging #8 into develop will decrease coverage by 23.37%.
The diff coverage is 22.22%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop       #8       +/-   ##
============================================
- Coverage    40.74%   17.37%   -23.38%     
============================================
  Files           49       43        -6     
  Lines         1885     1370      -515     
  Branches        28        0       -28     
============================================
- Hits           768      238      -530     
- Misses        1114     1132       +18     
+ Partials         3        0        -3
Impacted Files Coverage Δ
iCookTV/Views/MenuView.swift 0% <ø> (ø) ⬆️
iCookTV/Controllers/CategoriesViewController.swift 0% <ø> (-55.39%) ⬇️
iCookTV/Views/EmptyStateView.swift 0% <ø> (ø) ⬆️
iCookTV/Controllers/LaunchViewController.swift 79.45% <ø> (-16.06%) ⬇️
iCookTV/Views/MainMenuView.swift 0% <ø> (-77.42%) ⬇️
iCookTV/Views/SectionHeaderView.swift 0% <ø> (ø) ⬆️
iCookTV/Controllers/HistoryViewController.swift 0% <0%> (ø) ⬆️
iCookTV/Controllers/VideosViewController.swift 0% <0%> (ø) ⬆️
iCookTV/Controllers/VideoPlayerController.swift 0% <0%> (ø) ⬆️
iCookTV/Extensions/UIViewController+Alert.swift 75% <66.66%> (+75%) ⬆️
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c9832d...01569e7. Read the comment docs.

@bcylin bcylin force-pushed the feature/remove-rswift branch from 01569e7 to fdd1b38 Compare August 6, 2019 22:40
@bcylin bcylin closed this Aug 10, 2019
@bcylin bcylin deleted the feature/remove-rswift branch August 10, 2019 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants