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

Update factories.rst #3770

Closed
wants to merge 3 commits into from
Closed

Update factories.rst #3770

wants to merge 3 commits into from

Conversation

AlaaAttya
Copy link
Contributor

your factory method should be static to work!

your factory method should be static to work!
@wouterj
Copy link
Member

wouterj commented Apr 7, 2014

Indeed, the method needs to be static when setting the factory class and it can be non-static when setting a factory service

@weaverryan
Copy link
Member

No, it's not quite right yet, unfortunately. If you look at line 80:

When you specify the class to use for the factory (via factory_class)
the method will be called statically. If the factory itself should be instantiated
and the resulting object's method called (as in this example), configure the
factory itself as a service:

So, we're using this one example to explain both factory_class and factory_service. I think we should include this change, but reword this paragraph to something like:

When you specify the class to use for the factory (via factory_class)
the method will be called statically. If the factory itself should be instantiated
and the resulting object's method called, configure the factory itself as a service.
In this case, the method (e.g. get) should be changed to be non-static:

@AlaaAttya Can you make this change please? :)

Thanks!

@AlaaAttya
Copy link
Contributor Author

@weaverryan done :)

@weaverryan
Copy link
Member

Awesome, thanks so much @AlaaAttya!

weaverryan added a commit that referenced this pull request May 2, 2014
This PR was submitted for the 2.4 branch but it was merged into the 2.3 branch instead (closes #3770).

Discussion
----------

Update factories.rst

your factory method should be static to work!

Commits
-------

97ed5ce Update factories.rst
aefc36c Update factories.rst
a9cfdb2 Update factories.rst
@weaverryan weaverryan closed this May 2, 2014
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