Skip to content
This repository has been archived by the owner on Mar 27, 2018. It is now read-only.

Constantize: change ';' into constant RS as RootSeparator #29

Merged
merged 1 commit into from
Aug 5, 2015

Conversation

1e1
Copy link
Contributor

@1e1 1e1 commented May 9, 2014

RS is choose for RootSeparator and is as long as the DS (Directory) and PS (Path) constants.
It should be more human readable for contributors ;)

REM: Checktyle for the ending space of " * Read from stream"@Protocol.php:763

@1e1
Copy link
Contributor Author

1e1 commented May 9, 2014

Please test on a large project by replacing the ; as static::_define('RS', ';');@Core.php:143 by ̀µ (for example) and check.

@Hywan Hywan self-assigned this May 9, 2014
@Hywan
Copy link
Member

Hywan commented May 9, 2014

Hello :-),

We must create 2 constants here: ROOT_SEPARATOR and its alias RS, because Hoa\Core defines constants if it has not been previously defined before, so it's better to have long and short names.

@@ -135,33 +135,35 @@ class Core implements Parameter\Parameterizable {
*/
private function __construct ( ) {

static::_define('SUCCEED', true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The second parameters are right moved (+1 more indentation).

@Hywan
Copy link
Member

Hywan commented May 27, 2014

ping?

@1e1
Copy link
Contributor Author

1e1 commented May 27, 2014

pong!
Ok, I updated.

@Hywan
Copy link
Member

Hywan commented Aug 4, 2015

Please, rebase so that I can merge :-). Sorry for the delay…

@1e1 1e1 force-pushed the RootSeparator branch 5 times, most recently from 21b5bd4 to c70f7b1 Compare August 4, 2015 09:08
@1e1
Copy link
Contributor Author

1e1 commented Aug 4, 2015

Done. Ready for the code review.

@Bhoat Bhoat merged commit c70f7b1 into hoaproject:master Aug 5, 2015
@Hywan Hywan removed the in progress label Aug 5, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants