syndesisio / syndesis-integration-runtime

Apache License 2.0
1 stars 6 forks source link

feat: Extensible step kinds via service locator pattern #4

Closed jimmidyson closed 6 years ago

jimmidyson commented 6 years ago

This PR is a merge of funktionio/funktion-connectors#222 that makes more sense for Syndesis than it did for the original project.

I'd like to merge this ASAP so I can work on connector triggers in the runtime.

pure-bot[bot] commented 6 years ago

Pull request approved by @zregvart - applying approved label

jimmidyson commented 6 years ago

Hold off merging please - seen some stuff that needs updating... sorry! Dismissing PR review.

zregvart commented 6 years ago

TBH I would prefer two PRs one for the Service loading other for the new Step implementations added.

jimmidyson commented 6 years ago

I don't think this adds any new step implementations, just refactors existing step implementations to the new service loader mechanism. But it's been a long time since I did this originally and I could be wrong or misunderstanding what you mean - can you point out what you mean please?

zregvart commented 6 years ago

@jimmidyson from the diff at least it looks like io.syndesis.integration.runtime.stephandlers is a new package, I have misunderstood those for new steps, its part of the Service loader implementation, right?

jimmidyson commented 6 years ago

Yeah they are the extracted steps which were hard coded in the old route builder, no new functionality added, as you can see from the fact that tests were not updated, just simply refactoring.

jimmidyson commented 6 years ago

I decided to leave functionality as-is right now so this is simply a refactoring rather than changes to logic, which will happen in subsequent PRs. @zregvart If you want to re-review and approve, would be appreciated.

pure-bot[bot] commented 6 years ago

Pull request approved by @zregvart - applying approved label

jimmidyson commented 6 years ago

Thanks @zregvart!