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

Support for the Data Service Specification #738

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

laeubi
Copy link

@laeubi laeubi commented May 30, 2022

Add a first basic implementation of the DataSourceFactory

@gotson
Copy link
Collaborator

gotson commented Jul 28, 2022

@laeubi can you check the failing CI?

@laeubi
Copy link
Author

laeubi commented Jul 28, 2022

@gotson as it complains about code formatting is ther an Eclipse format profile or something I can use to format the source code?

@gotson
Copy link
Collaborator

gotson commented Jul 28, 2022

@gotson as it complains about code formatting is ther an Eclipse format profile or something I can use to format the source code?

You can use the Spotless plugin: mvn spotless:check and mvn spotless:apply

@laeubi laeubi force-pushed the issue_737 branch 2 times, most recently from 1f95d55 to 5ac503f Compare July 28, 2022 06:01
@laeubi
Copy link
Author

laeubi commented Jul 28, 2022

Done!

@gotson gotson self-assigned this Jul 28, 2022
@gotson gotson linked an issue Jul 29, 2022 that may be closed by this pull request
@gotson
Copy link
Collaborator

gotson commented Aug 1, 2022

This has no unit tests, is this feasible to add some ?

@laeubi
Copy link
Author

laeubi commented Aug 1, 2022

I'll try to add test-cases here, but have first to check how sqlite tests are structured.

@gotson
Copy link
Collaborator

gotson commented Aug 2, 2022

I've checked the issue you raised at osgi/osfgi-test and i think you framed it quite well: how to make it easier for maintainers that know nothing and have no particular interest in OSGI to gauge a PR and have confidence to merge it.

That probably will take a bit of time, but i think it's the right approach.

@laeubi
Copy link
Author

laeubi commented Aug 18, 2022

@gotson it hase taken a bit longer than desired but now I finally was able to add a small test-case, do you like to review and give some feedback if that would suffice?

@gotson
Copy link
Collaborator

gotson commented Aug 19, 2022

The more i think of it, the more i wonder whether this needs to be part of sqlite-jdbc. Could it not be part of a separate project that would offer that functionality only ? And if someone wants OSGi support they could add one more dependency in their pom.xml / build.gradle.

@laeubi
Copy link
Author

laeubi commented Aug 19, 2022

As always there are many options but in general this has several drawbacks, as already described here:

  1. The vendor has the best knowledge how to create and configure the participating objects.
  2. Changes in the internal implementation, e.g. refactoring of class names can be reflected an managed easier in the original repository, extra project maybe impose extra restrictions / problems (e.g. in this case a project needs to either embeds the sqlite-jdbc or republish it to add appropriate OSGi metadata)
  3. No gap between deployment of new version and connector code
  4. For non-osgi users, this is completely transparent and the is no need for them to change, but OSGi users will be happy to not reinvent the wheel, the library can just be dropped inside a framework and seaming less integrate, for example with the JPA Specification.

Beside that an extra project always needs to be found, needs extra release management and so on, also having "official" support let people directly contribute to the project instead of potentially contribute to two projects.

Add a first basic implementation of the DataSourceFactory
@laeubi
Copy link
Author

laeubi commented Aug 19, 2022

It seems I missed formatting the test code, fixed now ...

@gotson gotson removed their assignment Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please support the Data Service Specification for JDBC™ Technology
2 participants