Fixing typos, PHPDocs #2170

Merged
merged 8 commits into from Mar 7, 2013

Conversation

3 participants
Contributor

Borales commented Mar 5, 2013

I'm not sure about this PR, but I've decided to clean some typos and fix PHPDoc comments. If you have comments - I will be glad to hear them.

samdark was assigned Mar 5, 2013

@samdark samdark and 1 other commented on an outdated diff Mar 5, 2013

framework/base/CApplication.php
@@ -289,10 +289,11 @@ public function getExtensionPath()
return Yii::getPathOfAlias('ext');
}
- /**
- * Sets the root directory that holds all third-party extensions.
- * @param string $path the directory that contains all third-party extensions.
- */
+ /**
@samdark

samdark Mar 5, 2013

Owner

Why it's removed-added? Spaces-tabs problem? Make sure you've used tabs. Same for almost all changes.

@Borales

Borales Mar 5, 2013

Contributor

I don't know exactly why. I'm using phpStorm, it uses tabs.
I will recheck all other changes.

@samdark samdark and 1 other commented on an outdated diff Mar 5, 2013

framework/collections/CAttributeCollection.php
@@ -208,6 +208,6 @@ public function mergeWith($data,$recursive=true)
$d[strtolower($key)]=$value;
return parent::mergeWith($d,$recursive);
}
- parent::mergeWith($data,$recursive);
+ parent::mergeWith($data,$recursive);
@samdark

samdark Mar 5, 2013

Owner

What was wrong here?

@samdark

samdark Mar 5, 2013

Owner

You've changed tabs to spaces there. Should be tabs.

Owner

samdark commented Mar 5, 2013

Overall it's a good PR but there are issues with formatting to be fixed.

Contributor

Borales commented Mar 5, 2013

@samdark, um... I returned back the spaces with my changes, but it's still complicated to see the real diff..

Contributor

Borales commented Mar 5, 2013

@samdark ignore my previous comment, I think it became better now 😄 https://github.com/yiisoft/yii/pull/2170/files

@samdark samdark commented on an outdated diff Mar 5, 2013

framework/base/CApplication.php
@@ -292,6 +292,7 @@ public function getExtensionPath()
/**
* Sets the root directory that holds all third-party extensions.
* @param string $path the directory that contains all third-party extensions.
+ * @throws CException if the directory does not exist.
@samdark

samdark Mar 5, 2013

Owner

Space is used instead of tab. Should be one tab and one space.

@samdark samdark commented on an outdated diff Mar 5, 2013

framework/base/CComponent.php
@@ -248,6 +248,7 @@ public function __unset($name)
* to implement the behavior feature.
* @param string $name the method name
* @param array $parameters method parameters
+ * @throws CException if current class and its behaviors do not have a method or closure with given name
@samdark

samdark Mar 5, 2013

Owner

Space is used instead of tab. Should be one tab and one space.

Owner

samdark commented Mar 5, 2013

If you're using PhpStorm, try View → Active Editor → Show Whitespaces.

Contributor

Borales commented Mar 6, 2013

@samdark it should be ok now

@mdomba mdomba and 1 other commented on an outdated diff Mar 6, 2013

framework/base/CApplication.php
@@ -270,7 +270,7 @@ public function getRuntimePath()
/**
* Sets the directory that stores runtime files.
* @param string $path the directory that stores runtime files.
- * @throws CException if the directory does not exist or is not writable
+ * @throws CException if the directory does not exist or is not writable.
@mdomba

mdomba Mar 6, 2013

Member

please check all your changes for the @throw lines... somewhere you put the dot in the end, somewhere not...

as all the other @ statements like @Param, @return, etc. do not have the dot... then it makes sense to remove the dot for all @throw's too.

@Borales

Borales Mar 6, 2013

Contributor

@mdomba, sure, I will fix it.
But, as you can see yourself (in original scripts) - such statements (@) end with different ways (with dots and without them).
I will remove all my dots

@mdomba

mdomba Mar 6, 2013

Member

in general if there is only one sentence, there should be no dot...

only if there are two or more sentence for the same @ line then the last sentence ends with a dot.

@samdark samdark commented on an outdated diff Mar 6, 2013

framework/base/CComponent.php
@@ -248,6 +248,7 @@ public function __unset($name)
* to implement the behavior feature.
* @param string $name the method name
* @param array $parameters method parameters
+ * @throws CException if current class and its behaviors do not have a method or closure with given name
@samdark

samdark Mar 6, 2013

Owner

with the given name

@samdark samdark commented on an outdated diff Mar 6, 2013

framework/db/CDbCommand.php
@@ -200,6 +200,7 @@ public function getPdoStatement()
* this may improve performance.
* For SQL statement with binding parameters, this method is invoked
* automatically.
+ * @throws CDbException if CDbCommand failed to prepare the SQL statement.
@samdark

samdark Mar 6, 2013

Owner

Spaces?

@samdark samdark commented on an outdated diff Mar 6, 2013

framework/db/CDbCommand.php
@@ -463,6 +464,7 @@ public function queryColumn($params=array())
* you cannot bind parameters or values using {@link bindParam} or {@link bindValue}, and vice versa.
* Please also note that all values are treated as strings in this case, if you need them to be handled as
* their real data types, you have to use {@link bindParam} or {@link bindValue} instead.
+ * @throws CDbException if CDbCommand failed to execute the SQL statement.
@samdark

samdark Mar 6, 2013

Owner

Spaces?

@samdark samdark commented on an outdated diff Mar 6, 2013

framework/db/CDbCommand.php
@@ -549,6 +551,7 @@ private function queryInternal($method,$mode,$params=array())
* query options are supported: {@link select}, {@link distinct}, {@link from},
* {@link where}, {@link join}, {@link group}, {@link having}, {@link order},
* {@link limit}, {@link offset} and {@link union}.
+ * @throws CDbException if "from" key is not present in given query parameter.
@samdark

samdark Mar 6, 2013

Owner

Spaces?

@samdark samdark commented on an outdated diff Mar 6, 2013

framework/db/ar/CActiveFinder.php
@@ -181,6 +181,8 @@ private function destroyJoinTree()
* @param CJoinElement $parent the parent tree node
* @param mixed $with the names of the related objects relative to the parent tree node
* @param array $options additional query options to be merged with the relation
+ * @throws CDbException if given parent tree node is instance of {@link CStatElement}
@samdark

samdark Mar 6, 2013

Owner

is an instance

@samdark samdark commented on an outdated diff Mar 6, 2013

framework/db/ar/CActiveFinder.php
@@ -181,6 +181,8 @@ private function destroyJoinTree()
* @param CJoinElement $parent the parent tree node
* @param mixed $with the names of the related objects relative to the parent tree node
* @param array $options additional query options to be merged with the relation
+ * @throws CDbException if given parent tree node is instance of {@link CStatElement}
+ * or relation is not defined in given parent's tree node model class
@samdark

samdark Mar 6, 2013

Owner

in the given

@samdark samdark commented on an outdated diff Mar 6, 2013

framework/db/schema/CDbCommandBuilder.php
@@ -550,6 +551,7 @@ public function createPkCondition($table,$values,$prefix=null)
* This is only used when the third parameter is a string (query condition).
* In other cases, please use {@link CDbCriteria::params} to set parameters.
* @param string $prefix column prefix (ended with dot). If null, it will be the table name
+ * @throws CDbException if specified column is not found in given table.
@samdark

samdark Mar 6, 2013

Owner

Spaces?

@samdark samdark commented on the diff Mar 6, 2013

framework/gii/CCodeModel.php
@@ -291,6 +291,7 @@ public function getTemplatePath()
/**
* @param CCodeFile $file whether the code file should be saved
+ * @return bool
@samdark

samdark Mar 6, 2013

Owner

Description missing?

Contributor

Borales commented Mar 6, 2013

I'm sorry guys for making such mess. I'll try to do it cleaner next time.

samdark merged commit fa787ce into yiisoft:master Mar 7, 2013

Owner

samdark commented Mar 7, 2013

All good now. Thanks for working on it.

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