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

Documentation - Use nullable return type #9268

Merged
merged 1 commit into from
Apr 11, 2018
Merged

Documentation - Use nullable return type #9268

merged 1 commit into from
Apr 11, 2018

Conversation

Holicz
Copy link
Contributor

@Holicz Holicz commented Mar 15, 2018

In doctrine config the field flag is defined as nullable, so we should use nullable type in getter.

Q A
Branch? 1.0
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
License MIT

@Zales0123 Zales0123 added the Documentation Documentation related issues and PRs - requests, fixes, proposals. label Mar 16, 2018
pamil
pamil previously requested changes Mar 16, 2018
@@ -61,7 +61,7 @@ Assuming that you would want to add another field on the model - for instance a
/**
* @return bool
Copy link
Contributor

Choose a reason for hiding this comment

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

bool|null

@lchrusciel lchrusciel dismissed pamil’s stale review March 23, 2018 09:53

Comment applied

@lchrusciel
Copy link
Member

To which branch should it be merged? 1.1 or 1.0? /cc @Zales0123

@Zales0123
Copy link
Member

You're right, probably it should be 1.0 👍

@CoderMaggie CoderMaggie changed the base branch from 1.1 to 1.0 April 4, 2018 08:11
In doctrine config the field flag is defined as nullable, so we should use nullable type in getter.
@pamil pamil merged commit 6a0673d into Sylius:1.0 Apr 11, 2018
@pamil
Copy link
Contributor

pamil commented Apr 11, 2018

Thank you, Lukas! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation related issues and PRs - requests, fixes, proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants