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

Swift 4 migration #210

Merged
merged 37 commits into from
Jan 18, 2018
Merged

Swift 4 migration #210

merged 37 commits into from
Jan 18, 2018

Conversation

Ostrenkiy
Copy link
Contributor

Задача: #APPS-1663

Коротко для Release Notes, в формате «Сделали/Добавили/Исправили N»:
Миграция на swift 4

Описание:

  • Миграция на swift 4 всех таргетов
  • Исправлены версии подов (перед тем как смотреть pod update надо сделать)
  • Мимоходом убрал некоторые deprecation-ы

@Ostrenkiy Ostrenkiy added this to the 1.50 milestone Dec 21, 2017
@Ostrenkiy Ostrenkiy self-assigned this Dec 21, 2017
@Ostrenkiy Ostrenkiy requested a review from kvld December 21, 2017 01:08
@Ostrenkiy
Copy link
Contributor Author

Не будем вмердживать, пока не будет поддерживаться iPhone X и не настроится TeamCity.
Поддержку iPhone X отбранчую отсюда.

@kvld
Copy link
Contributor

kvld commented Dec 21, 2017

@Ostrenkiy ок. А это уже закончено, могу смотреть?

@Ostrenkiy
Copy link
Contributor Author

да, посмотри, не пропустил ли я чего
ну и потестить хорошо бы, я потыкался - вроде норм, но не все протыкивал

Copy link
Contributor

@kvld kvld left a comment

Choose a reason for hiding this comment

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

Убираем #if swift(>=3.2)?

Мы только заставляем компилироваться код под 4? Переписывать какие-то вещи не будем? Например, наболее интересные для нас (кажется): JSONEncoder/JSONDecoder (SE-0166) и модификатор private с extension (SE-0169).

@@ -171,7 +171,7 @@ class ExplorePresenter: CourseListCountDelegate {

private func getCachedLists(forLanguage language: ContentLanguage) -> [CourseList] {
let recoveredIds = courseListsCache.get(forLanguage: language)
return CourseList.recover(ids: recoveredIds).sorted { $0.0.position < $0.1.position }
return CourseList.recover(ids: recoveredIds).sorted { $0.position < $1.position }
Copy link
Contributor

Choose a reason for hiding this comment

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

Это фикс чего-то? Почему от $0.0 и $0.1 перешли к $0 и $1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

До этого в sorted передавался 1 параметр тьюпл, а теперь просто два параметра

@@ -281,7 +281,7 @@ class ExplorePresenter: CourseListCountDelegate {
if ContentLanguage.sharedContentLanguage != language {
throw LanguageError.wrongLanguageError
}
let newLists = lists.sorted { $0.0.position < $0.1.position }
let newLists = lists.sorted(by: { $0.position < $1.position })
Copy link
Contributor

Choose a reason for hiding this comment

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

Можно же sorted { ... }
И то же самое, что на 174 строке.

@Ostrenkiy
Copy link
Contributor Author

Я тут все поправил, кстати

@kvld
Copy link
Contributor

kvld commented Dec 28, 2017

А давай ещё NewProfile и NewSettings лишим префикса New? :)

@Ostrenkiy
Copy link
Contributor Author

Сейчас с New разберусь и мерджить в dev можно, думаю?

@Ostrenkiy
Copy link
Contributor Author

@kvld hey

@kvld
Copy link
Contributor

kvld commented Jan 18, 2018

@Ostrenkiy мерджи. TeamCity уже обновлен?

@Ostrenkiy Ostrenkiy merged commit fb70daa into dev Jan 18, 2018
@Ostrenkiy Ostrenkiy modified the milestones: 1.50, 1.51 Jan 28, 2018
@Ostrenkiy Ostrenkiy mentioned this pull request Jan 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants