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

@FormBuilder #2242

Merged
merged 11 commits into from
Oct 4, 2023
Merged

@FormBuilder #2242

merged 11 commits into from
Oct 4, 2023

Conversation

RomanPodymov
Copy link
Contributor

Hello.
Thank you for Eureka.
Add @FormBuilder.

README.md Outdated Show resolved Hide resolved
README.md Outdated
section.tag = "Section_B"
}
}
nil as TextRow?
Copy link
Member

Choose a reason for hiding this comment

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

What does this do?

Copy link
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
Fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Hi, so my question is why do we need to put a nil there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about if something { nil } ?

Copy link
Member

Choose a reason for hiding this comment

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

But why would you want to add a nil row or section? It is not clear to me what this would produce nor why would you use it like that

Copy link
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. So, should we remove it from the examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or you can add your suggestions to my branch.

Copy link
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
Can you please tell me what else should I fix in this pull request?

Copy link
Member

Choose a reason for hiding this comment

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

Hey @RomanPodymov I think if you remove the if something { nil} from the examples then this is fine to be merged

Copy link
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
Done.

@RomanPodymov RomanPodymov requested a review from mats-claassen May 8, 2023 14:04
@RomanPodymov
Copy link
Contributor Author

Hello @mats-claassen
What other changes should I make in this pull request?

@mlorenze mlorenze merged commit a77db05 into xmartlabs:master Oct 4, 2023
@RomanPodymov RomanPodymov deleted the feature/result_builders branch October 4, 2023 14:30
RomanPodymov added a commit to RomanPodymov/RomanPodymov that referenced this pull request Oct 4, 2023
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.

3 participants