Skip to content

Cleanup #65

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

Merged
merged 1 commit into from
Mar 3, 2016
Merged

Cleanup #65

merged 1 commit into from
Mar 3, 2016

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Mar 3, 2016

This will address some of the changes in #64

@@ -37,7 +37,6 @@ public function find($type)
{
$class = $this->findOneByType($type);

// TODO: use doctrine instantiator?
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 do not believe it is a good idea to instanciate a factory class without calling its constructor. Our DiactorosMessageFactory will not work unless its constructor is used.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, remove that comment

@sagikazarmark
Copy link
Member

Added a few more things to #64

@Nyholm
Copy link
Member Author

Nyholm commented Mar 3, 2016

Added changelog

@Nyholm
Copy link
Member Author

Nyholm commented Mar 3, 2016

👍

@sagikazarmark
Copy link
Member

I added a few things as well. Can you please squash commits? Call it something like Prepare stable release. Thank you.

@Nyholm
Copy link
Member Author

Nyholm commented Mar 3, 2016

Squashed!

@sagikazarmark
Copy link
Member

Thanks

sagikazarmark added a commit that referenced this pull request Mar 3, 2016
Cleanup before stable release
@sagikazarmark sagikazarmark merged commit 36e020c into master Mar 3, 2016
@sagikazarmark sagikazarmark deleted the cleanup branch March 3, 2016 21:10
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.

2 participants