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.

Sign up for GitHub

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

Jump to bottom

[SPT-1572] intro in macro #231

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from
Draft

[SPT-1572] intro in macro #231

wants to merge 13 commits into from

Conversation

NullIsOne
Copy link
Collaborator

@NullIsOne NullIsOne commented Jun 19, 2023 •
edited
Loading

Что сделано?

  • Написан макрос Mutable добавляющий структуре
  • Подключен макро-таргет в package ReactiveDataComponents
  • Подключен макро-таргет в основной пример на Xcodegen
  • ...

Зачем это сделано?

Для освоения макросов и скрытия однотипного кода в компонентах.

На что обратить внимание?

  • В тестах expansion не срабатывает, хотя по месту использования макрос генерирует функции как и ожидается
  • Рабочие конфигурации
    • SPM project + Components + Macro
    • Xcodegen + Example + Macro (привычный workspace)
  • Нерабочие конфигурации
    • Cocoapods project
  • Code coverage был отключен из-за проблем с macro таргетом
  • Стоит попробовать понизить iOS хотя бы до 12 (минимально поддерживаемая версия в Xcode 15)
  • Надо еще написать макрос генерирующий тело структуры Property
  • ...

Как протестировать?

  • Тапнуть в примере на строчку "Table with custom components"
  • Пример не крашится и визуально такой же как в предыдущем ПР

@NullIsOne NullIsOne requested review from Ikeret and Golubeykov June 19, 2023 05:57
@NullIsOne NullIsOne self-assigned this Jun 19, 2023
@github-actions
Copy link

github-actions bot commented Jun 19, 2023 •
edited
Loading

Warnings
⚠️

The .yml file has been modified. Keep changelog.md up to date.
⚠️ Oops! We have found some issues. It's better to fix them to keep code clean

SwiftLint found issues

Severity File Reason
Warning Decl+Extensions.swift:37 Lines should not have trailing whitespace. (trailing_whitespace)
Warning Decl+Extensions.swift:22 Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
Warning BuildableMacro.swift:75 Force unwrapping should be avoided. (force_unwrapping)
Error BuildableMacro.swift:16 Line should be 145 characters or less: currently 277 characters (line_length)
Error BuildableMacro.swift:20 Line should be 145 characters or less: currently 231 characters (line_length)
Warning BuildableMacro.swift:51 Lines should not have trailing whitespace. (trailing_whitespace)
Error MutableMacro.swift:8 Line should be 145 characters or less: currently 231 characters (line_length)
Warning MutableMacro.swift:41 Lines should not have trailing whitespace. (trailing_whitespace)
Warning MessageView.swift:253 Line should be 145 characters or less: currently 151 characters (line_length)
Warning MessageView.swift:123 Lines should not have trailing whitespace. (trailing_whitespace)

Generated by 🚫 Danger Swift against 1a5c5ad

@NullIsOne NullIsOne added the 8.0 label Jun 19, 2023
@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch from 5a76a3e to 2251ecc Compare June 19, 2023 07:04
Golubeykov
Golubeykov reviewed Jun 19, 2023
View reviewed changes
Example/project.yml
@@ -8,19 +8,14 @@ include:
name: ReactiveDataDisplayManagerExample
options:
# Версия Xcode
xcodeVersion: 13.4.1
xcodeVersion: 15.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Это что-то из локального сюда зашло? Как понимаю, только ты пробовал в XCode 15)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

это намеренно)
макросы требуют Swift 5.9
Swift 5.9 только в Xcode 15

короче эта часть изменений будет долго висеть.

Golubeykov
Golubeykov reviewed Jun 19, 2023
View reviewed changes
Copy link
Contributor

@Golubeykov Golubeykov left a comment

Choose a reason for hiding this comment

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

Предлагаю добавить в описании, что это история для XCode 15) А то я не сразу понял, почему не запускается)

Ikeret
Ikeret previously approved these changes Jun 19, 2023
View reviewed changes
Copy link
Contributor

@Ikeret Ikeret left a comment

Choose a reason for hiding this comment

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

У меня все работает, сам макрос даже читаемый получился, но можно еще описание добавить в ///)

Base automatically changed from feature/SPT-1476/componets_model_builder to develop June 19, 2023 11:06
@NullIsOne NullIsOne dismissed Ikeret’s stale review June 19, 2023 11:06

The base branch was changed.

@NullIsOne
Copy link
Collaborator Author

Обновил MutableMacroTests
Теперь все кейсы теста корректно отрабатывают

@NullIsOne NullIsOne changed the title Feature/spt 1476/macro intro in macro Jun 20, 2023
@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch 3 times, most recently from efaeed3 to 838f392 Compare June 20, 2023 12:51
@NullIsOne
Copy link
Collaborator Author

Перевожу CI на Xcode 15 раз уж в Gihub Actions добавили его, а тут

{ platform:iOS, id:dvtdevice-DVTiPhonePlaceholder-iphoneos:placeholder, name:Any iOS Device, error:iOS 17.0 is not installed. To use with Xcode, first download and install the platform }

Не до конца установили (Не поставили симулятор)

Ikeret
Ikeret reviewed Jun 21, 2023
View reviewed changes
Components/Macro/Package.swift
import CompilerPluginSupport

let package = Package(
name: "Macro",
Copy link
Contributor

@Ikeret Ikeret Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Какое то уникальное название надо придумать)

@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch from 838f392 to 8b9cee4 Compare July 20, 2023 11:49
@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch from 8b9cee4 to 81e0140 Compare July 25, 2023 11:20
@NullIsOne NullIsOne changed the title intro in macro [SPT-1572] intro in macro Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Golubeykov Golubeykov Golubeykov left review comments

@Ikeret Ikeret Ikeret left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@NullIsOne NullIsOne

Labels
8.0 enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NullIsOne @Ikeret @Golubeykov