Skip to content

Conversation

greg0ire
Copy link
Member

No description provided.

@greg0ire
Copy link
Member Author

The complicated part wasn't extracting the tokenizer, it was making Token a value object

goetas
goetas previously approved these changes Apr 1, 2020
Copy link
Member

Choose a reason for hiding this comment

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

do we really need this method?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's needed for tests/performance.php, and unit tests, but apart from that, not really, now. Doesn't really hurt though, does it?

Copy link
Member

Choose a reason for hiding this comment

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

Reminder: i still think that this param should go away and be passes in the specific method when possible

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not forgetting, I have #25 as a reminder ;)

@goetas
Copy link
Member

goetas commented Apr 1, 2020

to me looks good, I've just left some minor notes

Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

👍

@greg0ire greg0ire merged commit 94fedae into doctrine:master Apr 1, 2020
@greg0ire greg0ire deleted the extract-tokenization branch April 1, 2020 19:59
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