Skip to content

add support for multiple service annotations#238

Open
wodka wants to merge 3 commits into
schmittjoh:masterfrom
wodka:service-multiple
Open

add support for multiple service annotations#238
wodka wants to merge 3 commits into
schmittjoh:masterfrom
wodka:service-multiple

Conversation

@wodka
Copy link
Copy Markdown
Contributor

@wodka wodka commented Feb 14, 2016

this only adds the ability to define multipe service annotations on a class

to fully implement #232 we need to define how to add injections based on different service notations.

@wodka
Copy link
Copy Markdown
Contributor Author

wodka commented Feb 14, 2016

perhaps something like:

@Service(
  "first.service", 
  initMethod="setup", 
  initMethodInject=@InjectParams({
    "system" =@Inject("%first%)
  })
)
@Service(
  "second.service", 
  initMethod = "setup", 
  initMethodInject = @InjectParams({
    "system" = @Inject("%second%)
  })
)

Ideas?

*/
public function addService(array $service)
{
if (empty($this->id)) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IMO we should deprecate all this attributes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added the deprecated annotation to old properties

Comment thread Metadata/ClassMetadata.php Outdated
/**
* get list of defined services, use fallback of original fields
*
* return array[]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@return

@wodka wodka force-pushed the service-multiple branch 4 times, most recently from 5275158 to 6379051 Compare February 16, 2016 15:22
add deprecation warnings
add services support to AfterSetup
add tests
fix schmittjoh#232
@wodka
Copy link
Copy Markdown
Contributor Author

wodka commented Feb 16, 2016

@Ener-Getick I just added the support for AfterSetup services

Comment thread Metadata/MetadataConverter.php Outdated
);
}
foreach ($classMetadata->getServices() as $service) {
if (isset($environment)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

always true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use null !== instead, it is clearer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed it

@GuilhemN
Copy link
Copy Markdown
Collaborator

I'm wondering if we should not create a new ServiceMetadata class or just use the Definition class directly...

@GuilhemN
Copy link
Copy Markdown
Collaborator

@schmittjoh What do you think about having a new metadata class ? something like ServiceMetadata which would contain all the linked fields instead of having an array.

@GuilhemN
Copy link
Copy Markdown
Collaborator

as this is a big stuff, I think it will wait for 1.8.

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