Skip to content

API/SPI Combination and Examples #40

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
Apr 24, 2014
Merged

Conversation

benjchristensen
Copy link
Contributor

Subset of discussion at #37
Work as a result of discussion in #19

Subset of discussion at #37
Work as a result of discussion in #19
@jbrisbin
Copy link

+1

@benjchristensen
Copy link
Contributor Author

These changes were previously reviewed in #19 by @tmontgomery @jbrisbin @mariusaeriksen and @normanmaurer.

It was determined that we have general agreement and will then open new issues to discuss a few more things.

To move things along I'm going to consider #19 and #37 and the many +1s along the way sufficient approval to merge this and then open new issues to discuss naming, contracts, etc.

@benjchristensen
Copy link
Contributor Author

This completes #21

@benjchristensen
Copy link
Contributor Author

This completes #22

@benjchristensen
Copy link
Contributor Author

This completes #24

benjchristensen added a commit that referenced this pull request Apr 24, 2014
@benjchristensen benjchristensen merged commit b0b804a into master Apr 24, 2014
@benjchristensen benjchristensen deleted the combine-api-spi branch April 24, 2014 20:44
@benjchristensen
Copy link
Contributor Author

Further items of discussion off of this:

Contract => #41
Naming => #42
Subscription => #43
TCK => #39

@smaldini
Copy link
Contributor

Could that drive a 0.4 release granting that tck is removed ?

@rkuhn
Copy link
Member

rkuhn commented Apr 26, 2014

@smaldini: the TCK is the main product of this whole effort, so as long as we don’t have a TCK I don’t see how we can make a new release. Of course there is a circular dependency here:

  • first we settle on a set of interfaces
  • then everybody examines and validates them by creating implementations—at this point this means creating and consuming the SPI packages locally
  • these implementations need tests verifying the specified behavior, meaning that we collect a complete technology-agnostic set in the TCK
  • once we agree that the TCK is complete, we can make a release

We’ll have to do a variation of this process for every release cycle, but hopefully the set of changes will eventually converge to zero.

@benjchristensen
Copy link
Contributor Author

the TCK is the main product of this whole effort

I don't fully agree with this. In 15+ years of programming I have never once validated an interface implementation against a TCK, but I have often needed to read the interfaces and javadocs for what the contract needs to be so I can write a valid implementation.

Especially if we end up pursuing polyglot support (#45) with a network protocol, the contract is far more important as multiple languages will all implement against it.

Thus, I'd argue the main product of this is not the TCK, but the contract definition: #41

Therefore, I'd support a release of the current types after finishing #41 even if we don't have a TCK implemented yet.

The TCK will be valuable, but it is by no means the purpose or main product of this effort.

@jbrisbin
Copy link

I agree with @benjchristensen here. A Java TCK will be of limited benefit as I work on Javascript bindings and it's not really practical to implement a TCK for all the environments that Javascript runs in. IMO having the interfaces to model prototypes on, as well as having the contract defined is sufficient.

+1 for releasing 0.4.0 so we can plunge ahead with the work we're doing to implement Reactive Streams in Reactor (we're in limbo right now waiting on upcoming changes which are significantly different from 0.3.0).

@rkuhn
Copy link
Member

rkuhn commented Apr 28, 2014

You are right that a Java TCK is only useful for JVM-based implementations, and in light of the extension in scope that we are discussing in #45 I would like to extend “TCK” to include an automated verification mechanism for all parts that we release, be that implementations of the interfaces on other platforms or network protocols. Interoperability between different implementations is key to this effort, otherwise it would be sufficient to just publish a white paper proposing an algorithm for possible solutions instead of specifying interfaces and their semantics.

I find it very helpful to have a working and comprehensive test suite before any release also for the reason that writing down semantics is of course important, but without the test suite we don’t know whether there can be a conforming implementation—we might have contradicting requirements in the spec otherwise. It is tempting to forego that verification during the 0.x series of a project, but I am not sure that that will result in us getting to 1.0 faster; the danger is that we slip into wishful thinking, and a required test suite would serve to keep us honest (and I expressly include myself here, knowing how beneficial this was in my own past).

@jbrisbin
Copy link

Since we're still in flux with what the TCK will actually be testing, it seems more productive IMO to forge ahead with a "real" implementation (versus the stub implementations necessary in the TCK) so we can figure out what's actually necessary to test and verify.

I know @smaldini has Reactor 2.0's Reactive Streams implementation passing the 0.3.0 TCK but we're still in limbo as we're trying to anticipate the upcoming changes.

Is there anything still in the air for 0.4.0 that we can't start working on a TCK for the latest changes?

@benjchristensen
Copy link
Contributor Author

anything still in the air for 0.4.0

I think it's just agreeing on the contract in #41

krasserm referenced this pull request in krasserm/akka Apr 29, 2014
- include feedback
- more config parameters
   * idle time
   * fromSequenceNr
- user docs
- tests
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.

4 participants