-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Patch lbvs_consent 2.1.1-2.2.0 to support openbabel-3 #24677
Conversation
I somehow managed to include illegal white chars again... 🤦 I've therefore checked that all 4 four opam files lint locally.
This is unrelated to the changes in this PR though (which doesn't touch the license field) |
I see FreeBSD failures on
which should be solved by #24652 |
9c86a07 adds a lower bound on |
|
Overall there remains
I suggest to wait until #24652 #24679 and #24680 have been merged, and then rerun CI before merging this one. |
42ee938
to
62d687e
Compare
Rebased on |
There are still |
There are also still |
62d687e
to
3719e3e
Compare
CI revealed a missing bound on camlzip:
|
The remaining failures are all from |
In general I am against patches on released packages and ask to release a -1 patch version. In this case I would be in favour since it is a strict improvement over what we have and reduces drastically the failures. We need to discuss this at the next maintainer meeting and see if we all agree before we can merge |
Since this was merged upstream and it is a specialized software that would become available on a larger set of platforms, I am going to accept and merge the change |
Thanks and sorry for the delay |
The previous PR #24545 patched
conf-openbabel
to install openbabel-2 or 3.The
lbvs_consent
however still hardcodes an openbabel-2 include path which causes CI build errors, and thus adds noise:This follow-up PR thus patches the Makefiles for
lbvs_consent
2.1.1-2.2.0 to utilize #24545.I've filed an upstream PR with the corresponding change here: UnixJunkie/consent#15