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

Creating three packages and profiles for code generation #22

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

Conversation

charbull
Copy link
Collaborator

The current state of OLGA requires the users to install dotnet, and python dependencies which requires unnecessary effort specially when the user is for example interested only in Java code generation or Csharp.

This PR proposes the following:

  • bumps the OLGA versions
  • refactors the code generators into three places (java, csharp, python)
  • refactors the tests into three packages (java, csharp, python)
  • creates maven profiles which skip specific tests based on the selected profile

(two tests are not passing so far: testImportSaref and testMergedSaref) in OLGAJavaTest, commented for now, I will resolve the issue after this PR). # #

@charbull charbull added the enhancement New feature or request label Jul 21, 2020
@charbull charbull self-assigned this Jul 21, 2020
@charbull charbull linked an issue Jul 21, 2020 that may be closed by this pull request
@charbull
Copy link
Collaborator Author

@radissoa @AlberTadrous @joephayes any review :)

@charbull
Copy link
Collaborator Author

@AlberTadrous there are two tests that are failing but not sure why yet, as I didn't touch the java generator.
testImportSaref and testMergedSaref) in OLGAJavaTest
any hints? ideas?

@@ -28,6 +28,11 @@
import java.util.Map;
import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
import semanticstore.ontology.library.generator.exceptions.InvalidUriException;
import semanticstore.ontology.library.generator.generators.AbstractGenerator;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to make these plugins that are loaded at runtime? It would make it easier for a person adding a new generator...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joephayes
I actually started using Dagger for dependency injection as you are pointing out.
Then I realized others might not use it to extend OLGA.
I think it makes sense to seperate them and run the tests of code generation for all three.
But the compilation tests we can't assert unless the underlying dependencies are installed.

thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joephayes friendly ping :)

Copy link
Contributor

@joephayes joephayes Jul 30, 2020

Choose a reason for hiding this comment

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

@charbull Sorry for the delay!

At a high level, I'm thinking in terms of modules (plugins). OLGA would maintain a registry of modules and load the code as needed.

What if OLGA defined these as service interfaces and then the Python, C# and Java generators can be implemented as services? OLGA could then use the Java ServiceLoader to load the desired implementations.

If OLGA adopted this model, the code for the different generators would move into separate repos and the compilation tests would move as well.

It would also give a definitive answer to the question "how do I add a new generator?"

Copy link
Contributor

Choose a reason for hiding this comment

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

@charbull - is this something you want to discuss more? Maybe I should open a separate issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joephayes I think it makes sense, I will start looking at it.
For the RDF python generator do you have some bandwidth to collaborate in co-development?

Copy link
Contributor

@joephayes joephayes Aug 7, 2020

Choose a reason for hiding this comment

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

@charbull - A colleague of mine and I will have some bandwidth towards the end of this month and we could work on the generator for RDF Python then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's great !
Will send another PR for the service loader

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds great! Looking forward to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the pom.xml to exclude tests for any language/library
3 participants