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

Updated syntax for "integer" and "boolean" types #1457

Merged
merged 1 commit into from
Sep 5, 2015

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Jul 14, 2015

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Used short syntax for integer and boolean types.

Before

/**
 * @var integer
 *
 * @ORM\Column(name="some_integer_field", type="integer")
 */
private $someIntegerField;

/**
 * @var boolean
 *
 * @ORM\Column(name="some_boolean_field", type="boolean")
 */
private $someBooleanField;

After

/**
 * @var int
 *
 * @ORM\Column(name="some_integer_field", type="integer")
 */
private $someIntegerField;

/**
 * @var bool
 *
 * @ORM\Column(name="some_boolean_field", type="boolean")
 */
private $someBooleanField;

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Used short syntax for ```integer``` and ```boolean``` types.

**Before**
```php
/**
 * @var integer
 *
 * @Orm\Column(name="some_integer_field", type="integer")
 */
private $someIntegerField;

/**
 * @var boolean
 *
 * @Orm\Column(name="some_boolean_field", type="boolean")
 */
private $someBooleanField;
```

**After**
```php
/**
 * @var int
 *
 * @Orm\Column(name="some_integer_field", type="integer")
 */
private $someIntegerField;

/**
 * @var bool
 *
 * @Orm\Column(name="some_boolean_field", type="boolean")
 */
private $someBooleanField;
```
@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3824

We use Jira to track the state of pull requests and the versions they got
included in.

@TomasVotruba
Copy link
Contributor

This seems ready.

@holtkamp
Copy link
Contributor

holtkamp commented Sep 4, 2015

Just wondering, what is the added value of 'going back to abbreviations'? Preparing for PHP7 type hinting?

@TomasVotruba
Copy link
Contributor

@holtkamp Might be. Consistency improves readability and also prevents confusion. It would be also easier to adapt PHP 7 type hinting, as you state.

@holtkamp
Copy link
Contributor

holtkamp commented Sep 4, 2015

@TomasVotruba personally I regard abbreviations something that should only be used when needed (for example when there is insufficient space on a piece of paper, one uses an abbreviation). Alias functions like is_integer() triggered me to prefer the 'integer' over 'int' notation, hoping that one day we would also have an is_boolean() function.

About the consistency:

/**
 * @var bool
 *
 * @ORM\Column(name="some_boolean_field", type="boolean")
 */
private $someBooleanField;

I see 1x bool and 3x boolean...

Might be my personal glitch, but I would say 👎

@TomasVotruba
Copy link
Contributor

@holtkamp I have no opinion about this, it doesn't burn me. Just seems ready.

@deeky666
Copy link
Member

deeky666 commented Sep 5, 2015

@holtkamp these abbreviations are also used in the upcoming PSR-5: PHPDoc standard. So IMO this PR is a good thing.

@Ocramius Ocramius self-assigned this Sep 5, 2015
Ocramius added a commit that referenced this pull request Sep 5, 2015
Updated syntax for "integer" and "boolean" types
@Ocramius Ocramius merged commit a0a0c73 into doctrine:master Sep 5, 2015
@Majkl578
Copy link
Contributor

Majkl578 commented Sep 5, 2015

@holtkamp:

I see 1x bool and 3x boolean...

How often do you add type name into the variable name? That seems crazy. In real world, you'd have something like:

/**
 * @var bool
 * @ORM\Column(name="is_enabled", type="boolean")
 */
private $enabled;

1x bool and 1x boolean.

It's sad that Doctrine doesn't support both variants, but that's for another discussion.

@TomasVotruba
Copy link
Contributor

👍 Thanks guys

@Ocramius
Copy link
Member

Ocramius commented Sep 7, 2015

It's sad that Doctrine doesn't support both variants, but that's for another discussion.

Meh, we have more relevant stuff to deal with anyway.

@holtkamp
Copy link
Contributor

holtkamp commented Sep 8, 2015

@deeky666 aah, nice, did not know that PSR yet
@Majkl578 true
@Ocramius so true

I stand corrected 😄

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.

8 participants