-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Move JDBC connector deps to :dev alias #18
Move JDBC connector deps to :dev alias #18
Conversation
konserve-jdbc didn't have a dev profile, so I made one. FYI, the dependencies will obviously have to be added to the dev alias for every library that uses konserve-jdbc |
@TimoKramer @jsmassa Does that make sense to you? |
We'll need the drivers in the test-alias as well otherwise the tests won't run. |
And some update in the documentation would be good where you describe the breaking change and the need to add the drivers manually |
In theory you can use something like edit: nevermind, found it! I'm going to change
Good point! Would a note in README.md suffice? It would be nice to be included in the next release notes too. Also are you sure you're ok with introducing this breaking change? I suppose most users will provide their own jdbc driver(s) as direct dependencies, so few apps will actually break but you never know... |
right, I am hesitant to change the orb
Right, we'll add it prominently to the README and we'll put it in the commit message and we'll announce it on our slack channel.
|
Alright @TimoKramer , please check again, I think I've addressed everything! By the way I ran Apparently it's working for you so it shouldn't matter, it just means that I can't test that everything still works as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing you @PavlosMelissinos could change is to increase the minor release version in the build.clj to 0.2. |
Co-authored-by: Timo Kramer <4785848+TimoKramer@users.noreply.github.com>
Co-authored-by: Timo Kramer <4785848+TimoKramer@users.noreply.github.com>
@TimoKramer done, I think |
Merge? @whilo |
Thanks a lot for the PR! |
No description provided.