Skip to content

@SectionBuilder#2235

Merged
mats-claassen merged 5 commits intoxmartlabs:masterfrom
RomanPodymov:feature/section_builder
Nov 16, 2022
Merged

@SectionBuilder#2235
mats-claassen merged 5 commits intoxmartlabs:masterfrom
RomanPodymov:feature/section_builder

Conversation

@RomanPodymov
Copy link
Copy Markdown
Contributor

@RomanPodymov RomanPodymov commented Nov 10, 2022

Hello.
Thank you for Eureka.
Added the first @resultBuilder to the project. If you like it, I will add more.
@mats-claassen Or should I add some documentation for @SectionBuilder?

Copy link
Copy Markdown
Member

@mats-claassen mats-claassen left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the contribution. Looks great overall, just one comment

Comment thread Source/Core/ResultBuilders.swift Outdated
}

public static func buildExpression(_ expression: String?) -> [BaseRow] {
[.init(tag: expression)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This would be instantiating a BaseRow (not a concrete Row). Not sure about how this would be useful

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hello @mats-claassen
Exactly, deleted it.

@mats-claassen
Copy link
Copy Markdown
Member

Also, yes please add documentation to the Readme about this feature

@RomanPodymov
Copy link
Copy Markdown
Contributor Author

Also, yes please add documentation to the Readme about this feature

Done.

@mats-claassen mats-claassen merged commit ba4ebbc into xmartlabs:master Nov 16, 2022
@RomanPodymov RomanPodymov deleted the feature/section_builder branch November 17, 2022 09:28
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