fix several phpDoc and type hints #242

Merged
merged 1 commit into from Jul 9, 2012

5 participants

@havvg
Propel member

see #241

@travisbot

This pull request passes (merged 2b615322 into b51539a).

@willdurand
Propel member

@hhamon what is the best Boolean or boolean?

@stephpy

I guess that should be boolean because Boolean can be interpreted as an object.

@havvg
Propel member

Boolean will be recognized as its own class (from e.g. PHPStorm), but it does not exist. boolean is recognized as the internal type.
This also applies to Array, Integer and String.

@havvg
Propel member

Then a PR should be opened there, as well :-)

@willdurand
Propel member
@havvg
Propel member

It seems inconsistent, imho: they use array, string etc., but Boolean (and even boolean https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Application.php#L648).

@willdurand
Propel member
@stephpy

It depends about contributors, there is too boolean on Symfony2:

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/ParameterBag.php#L95

As @havvg said, i think it would be more readable to use lowercase for internal type, for ides and to be differs internal type from Classes.

@stephpy

I created an issue for that on Symfony2 repo, we may should create a psr but i did not found the time to create it

symfony/symfony#4183

@willdurand
Propel member

Guys, I got the reason, and we could change to boolean.

@havvg
Propel member

Please wait with the merge. I found some more (at least in Propel 1.6) and would like to check for those in Propel2 as well.

@havvg
Propel member

see propelorm/Propel#382 for more

@willdurand
Propel member

@havvg can you rebase this PR?

@havvg
Propel member

Will do, after adding the ones from 382 of Propel.

@hhamon
Propel member

Personally I would be in favor of boolean.

@willdurand
Propel member

@hhamon everything is ok then?

@hhamon hhamon commented on an outdated diff Jun 18, 2012
...l/Runtime/Connection/ConnectionManagerMasterSlave.php
@@ -43,7 +43,7 @@ class ConnectionManagerMasterSlave implements ConnectionManagerInterface
protected $readConnection;
/**
- * @var Boolean Whether a call to getReadConnection() alsways returns a write connection.
+ * @var boolean Whether a call to getReadConnection() alsways returns a write connection.
@hhamon
Propel member
hhamon added a note Jun 18, 2012

alsways => always

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@willdurand
Propel member

@havvg can you rebase your PR, and fix @hhamon's comment?

@havvg
Propel member

Done, plz wait for travis; I can't run the suite on my local machine right now :(

@travisbot

This pull request fails (merged e16149e5 into a6aa07d).

@willdurand
Propel member

Seems good, tests on 5.3.3 don't pass anyway. Thanks!

@willdurand willdurand merged commit b2ed04a into propelorm:master Jul 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment