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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

phpdoc in Table::addForeignKey() update #436

Merged
merged 2 commits into from Dec 13, 2013

Conversation

velosipedist
Copy link
Contributor

Added significant typehint in PhpDoc: $foreignTable param in addForeignKeyConstraint() can be just string name of table.

It unobvious in IDE hinting and treated as wrong param. It even can confuse developer, forcing him use foreign keys additions only using existing tables. That was my case, for example 馃槙 Lost time till found out, that Table schema not necessary to exist for Foreign Key referring .

@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/DBAL-692

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

@stof
Copy link
Member

stof commented Nov 29, 2013

some other methods are also accepting `Table|string``, so they should also be fixed

@velosipedist
Copy link
Contributor Author

ok, 'll try to fix .You mean other methods of Table only, or i should inspect all DBAL code?

@stof
Copy link
Member

stof commented Nov 29, 2013

I mean other methods of this class

@velosipedist
Copy link
Contributor Author

Checked whole project, all @param annotations

beberlei added a commit that referenced this pull request Dec 13, 2013
phpdoc in Table::addForeignKey() update
@beberlei beberlei merged commit fa74679 into doctrine:master Dec 13, 2013
@velosipedist velosipedist deleted the hotfix/PhpDoc branch December 14, 2013 11:52
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants