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

Fix fieldMapping phpdoc #8556

Merged
merged 1 commit into from Mar 26, 2021
Merged

Fix fieldMapping phpdoc #8556

merged 1 commit into from Mar 26, 2021

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet
Copy link
Contributor Author

Friendly ping @greg0ire for the review :)

* scale?:int,
* unique?: string,
* inherited?: class-string
* }>
Copy link
Member

Choose a reason for hiding this comment

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

Do you think this list is exhaustive now? If not please do this:

Suggested change
* }>
* }>&array<string,mixed>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know at all, I just followed the phpdoc:

     * The mapping definition array has the following values:
     *
     * - <b>fieldName</b> (string)
     * The name of the field in the Entity.
     *
     * - <b>type</b> (string)
     * The type name of the mapped field. Can be one of Doctrine's mapping types
     * or a custom mapping type.
     *
     * - <b>columnName</b> (string, optional)
     * The column name. Optional. Defaults to the field name.
     *
     * - <b>length</b> (integer, optional)
     * The database length of the column. Optional. Default value taken from
     * the type.
     *
     * - <b>id</b> (boolean, optional)
     * Marks the field as the primary key of the entity. Multiple fields of an
     * entity can have the id attribute, forming a composite key.
     *
     * - <b>nullable</b> (boolean, optional)
     * Whether the column is nullable. Defaults to FALSE.
     *
     * - <b>columnDefinition</b> (string, optional, schema-only)
     * The SQL fragment that is used when generating the DDL for the column.
     *
     * - <b>precision</b> (integer, optional, schema-only)
     * The precision of a decimal column. Only valid if the column type is decimal.
     *
     * - <b>scale</b> (integer, optional, schema-only)
     * The scale of a decimal column. Only valid if the column type is decimal.
     *
     * - <b>'unique'</b> (string, optional, schema-only)
     * Whether a unique constraint should be generated for the column.

The inherited key already added wasn't in the phpdoc.
I don't know if someone can confirm there is nothing else... If nobody can, I will add your suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's definitely incomplete. I think I added more in the 2.9.x branch:
https://github.com/doctrine/orm/blob/2.9.x/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php#L440

Also this PR should supersede #8302 (which was already kinda superseded by the PR on 2.9.x)

Copy link
Contributor

Choose a reason for hiding this comment

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

#8302 (comment) this one should be considered too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added

originalClass?: class-string,
originalField?: string

from the 2.9 branch

And also

quoted?: bool
requireSQLConversion?: bool

I found nothing else in the code

@greg0ire
Copy link
Member

@orklah please review.

@greg0ire greg0ire added the Bug label Mar 26, 2021
@greg0ire greg0ire merged commit c8f2f61 into doctrine:2.8.x Mar 26, 2021
@greg0ire
Copy link
Member

Thanks @VincentLanglet !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants