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

Refactor for extension #58

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Refactor for extension #58

wants to merge 8 commits into from

Conversation

aalmiray
Copy link
Contributor

WARNING: Big refactor ahead.

Motivation: VWorkflows 0.2.x makes it difficult to extend behavior for third parties as many of the classes expect specific implementations of a particular interface. There are also many places where Java's package private scope is used.

This pull request offers an alternate design. The library already defined most of its core types as interfaces while supplying default implementations. These implementations have been split into two packages: base and impl. The base package may be used as starting point for 3rd parties that would like to write their own custom implementations of the core interfaces. The impl package provide ready-made, default implementations of the core interfaces, in effect they are behavior compatible with version 0.2.x.

Usage of package private access has been reduced to a minimum.
Classes have been relocated to other packages, grouped by responsibility.

This refactor constitutes a break in binary compatibility

Please review at your earliest convenience. We're open to feedback and discussion on the proposed changes as we understand it may be too big to swallow in one go.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 17.756% when pulling 594d8d2 on aalmiray:factories into 0e1522c on miho:master.

@cbvms123
Copy link

This patch has a dependency to scaledfx 0.4 which is not yet uploaded to central.

@miho
Copy link
Owner

miho commented May 8, 2017

Huge, indeed :) Since it seems to introduce incompatibilities, I won't merge it right now. But we will meet in a few days anyway and we can certainly find some time for further discussions.

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