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

Made PATTERN_MATCH_ID_METHOD more flexible #350

Merged
merged 1 commit into from
Aug 31, 2015

Conversation

4alexandr
Copy link
Contributor

PATTERN_MATCH_ID_METHOD match any valid function declaration, example:
function getId(){return$this->id;}

PATTERN_MATCH_ID_METHOD match any valid function declaration, example:
function getId(){return$this->id;}
@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/DCOM-269

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

@Ocramius
Copy link
Member

@4alexandr requires test case.

@4alexandr
Copy link
Contributor Author

@Ocramius Sorry, i don't know, how to write tests for this change. I can test this only manually.
Old pattern can match

    public function getId()
    {
        return $this->id;
    }

but can't

    public function getId(){
        return $this->id;
    }

In commit i change non-required whitespaces detection in function declaration.

@Ocramius
Copy link
Member

@4alexandr look at existing tests for the proxy generator ;-)

@Ocramius
Copy link
Member

Is also fixed by something like doctrine/orm#1241 (linking for reference - this partial fix is fine for now)

@Ocramius Ocramius self-assigned this Mar 25, 2015
@Ocramius
Copy link
Member

Related (not blocking) doctrine/orm#1241

@Ocramius
Copy link
Member

Checked again, and writing a test case for this is a huge annoyance for little benefit. Merging as-is.

Ocramius added a commit that referenced this pull request Aug 31, 2015
Made PATTERN_MATCH_ID_METHOD more flexible
@Ocramius Ocramius merged commit 7d4f8e4 into doctrine:master Aug 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants