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

WIP: Split into subprojects - core and extras. #28

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

Conversation

alexanderkjeldaas
Copy link

Not for merging, but is this split reasonable?

@alexanderkjeldaas alexanderkjeldaas changed the title WIP: Split into subprojects. WIP: Split into subprojects - core and extras. Sep 28, 2018
@larsga
Copy link
Collaborator

larsga commented Sep 28, 2018

Yes, I think this looks entirely reasonable. I need to review the details in how the gradle files were split, but the rest looks very nice.

Regarding how to package the functions you can look at the experimental module: https://github.com/schibsted/jslt/blob/master/src/main/java/com/schibsted/spt/data/jslt/impl/ExperimentalModule.java

Someone wanting to use the module can pass it to the parser with this method https://github.com/schibsted/jslt/blob/master/src/main/java/com/schibsted/spt/data/jslt/Parser.java#L176

See the experimental module for the use of URIs to name modules.

The other open PR by @pratikpparikh has some service provider interface (SPI) concept in it that I think could be relevant. You can look into that if you want, or if you prefer we can just do the split and the new module first, and consider the SPI issue later.

@larsga
Copy link
Collaborator

larsga commented Nov 20, 2018

@alexanderkjeldaas Are you still working on this?

@pratikpparikh
Copy link

@larsga if @alexanderkjeldaas is not working on it can we just do it between us?

@larsga
Copy link
Collaborator

larsga commented Nov 20, 2018

The task itself is trivial to do. But if there isn't anything to add in the non-core part then we might as well wait. :-)

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.

3 participants