Skip to content

Conversation

@matej21
Copy link
Contributor

@matej21 matej21 commented Sep 13, 2015

@greeny
Copy link
Contributor

greeny commented Sep 13, 2015

Nice one! 👍

@fprochazka
Copy link
Contributor

Nice!

@dg
Copy link
Member

dg commented Oct 9, 2015

What about something like

services:
    articles: MyBlog\ArticlesModel(@connection)
    comments: MyBlog\CommentsModel(@connection, @self.articles)

@matej21
Copy link
Contributor Author

matej21 commented Oct 9, 2015

@dg maybe it looks a bit better, but this won't work. But it is probably useless anyway.

@dg
Copy link
Member

dg commented Oct 9, 2015

Hmmm, passing service names to methods seems like antipattern.

@matej21 matej21 force-pushed the feature/compiler_prefix branch 3 times, most recently from 7b4d0ec to b450498 Compare October 9, 2015 15:49
@matej21
Copy link
Contributor Author

matej21 commented Oct 9, 2015

updated

@dg
Copy link
Member

dg commented Oct 9, 2015

Great!

I'm just not quite sure that self is the best name for this prefix.

@matej21
Copy link
Contributor Author

matej21 commented Oct 9, 2015

I'm not sure either since it is used for self-referencing service.

btw, maybe this could be moved to the ContainerBuilder so you can use @self.foo syntax in the CompilerExtension as well (instead of $this->prefix('@foo'))

@greeny
Copy link
Contributor

greeny commented Oct 13, 2015

btw, maybe this could be moved to the ContainerBuilder so you can use @self.foo syntax in the CompilerExtension as well (instead of $this->prefix('@foo'))

Good idea 👍

What about @this, @current or @scope?

@dg
Copy link
Member

dg commented Oct 13, 2015

btw, maybe this could be moved to the ContainerBuilder so you can use @self.foo syntax in the CompilerExtension as well (instead of $this->prefix('@foo'))

I can imagine that it could cause complications, for example when somebody will addSetup('@self ...') to service from a different scope. In CompilerExtension you need prefix() only in addDefinition().

@dg
Copy link
Member

dg commented Jan 13, 2016

@current, @this and @self sound similary. @scope sound fine, but has a different meaning in DI http://www.javaranch.com/journal/2008/10/dependency-injection-what-is-scope.html.

@milo milo added this to the v2.4 milestone Mar 29, 2016
@dg dg force-pushed the master branch 2 times, most recently from 231a29c to 7f12a9f Compare April 21, 2016 13:03
@matej21 matej21 force-pushed the feature/compiler_prefix branch from b450498 to c2b87de Compare April 29, 2016 18:53
} elseif ($namespace) {
$name = $namespace . '.' . $name;
}
if ($namespace) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you move it bellow next condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@matej21 matej21 force-pushed the feature/compiler_prefix branch from c2b87de to 2a2be47 Compare April 29, 2016 19:00
@dg
Copy link
Member

dg commented Apr 29, 2016

Thanks!

@dg dg merged commit 5159e83 into nette:master Apr 29, 2016
@matej21
Copy link
Contributor Author

matej21 commented Apr 29, 2016

great! now I can finally fix the doc :)

dg pushed a commit that referenced this pull request Apr 29, 2016
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.

5 participants