-
Notifications
You must be signed in to change notification settings - Fork 53
cffi: update to use libyang v4.2.2 #160
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
base: master
Are you sure you want to change the base?
Conversation
|
This PR apparently needs to wait for official release of libyang v4.1.0. If you would like to test it by yourself in the mean time, use libyang |
|
What will happen if the underlying system hasn't upgraded to libyang v4? |
It won't work. As |
I think for our use we won't be able to require our users upgrade to 4. So I think the main problem is going to be making sure they get a working version of the python bindings. It might make sense to create a libyang3 stable branch to track critical fixes. Also not sure how to handle 2 versions of the bindings with pypi. Does pypi let you update multiple versions that vary by major? |
Frankly speaking, the requirement of having specific version or higher of Regarding the pypi versioning. In general updates are possible and can support multiple major versions, but it depends on maintainers, how many stable versions they want to support and all the staff along with it (backporting bugfixes....). |
|
My time for maintaining libyang-python is very limited. I will not be able to support maintenance/stable branches. When libyang 4.x is released, I will accept this PR and we can bump the python project version to v4.0.0 to follow libyang version scheme. |
rjarry
left a comment
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.
Could you squash all in a single commit? I want the git history to remain bisectable.
Also, could you use imperative mood in your commit message?
Lines 258 to 261 in ddb7e99
| - Describe your changes in imperative mood, e.g. *"make xyzzy do frotz"* | |
| instead of *"[This patch] makes xyzzy do frotz"* or *"[I] changed xyzzy to | |
| do frotz"*, as if you are giving orders to the codebase to change its | |
| behaviour. |
2f56b6a to
f2852c7
Compare
Done |
Perhaps I could offer to help? I'd prefer not to have to fork and do this alone, so a collaboration would be preferable. Maybe you could create a v3 stable branch and grant permissions to that, and I (for a time) could maintain it, doing the back-ports required (in FRR we use mergifyio to make this as painless as possible). That way v3 would still be updated and fetchable the same way from pypi. |
f2852c7 to
4ae29fc
Compare
Oh that would be much appreciated 👍 Sorry, I had missed your message and only see it now. I have invited you as a member of the project with write/push access. I have created a In order to create new releases on pypi.org, all you need to do is push a signed/annotated tag on that branch following the pattern libyang-python/.github/workflows/ci.yml Lines 94 to 114 in 3815807
Thanks for stepping up ❤️ |
Got it. Might need to do something with tox.ini/tox-install.sh to have it fetch the correct v3 of libyang too. I'll take a look at that.
Thanks for the invitation. :) |
|
Looks like libyang released last week: https://github.com/CESNET/libyang/releases/tag/v4.2.2 |
d71a3a4 to
d6faad4
Compare
I adjusted the patch to work with v4.2.2. Unfortunately there is still one test failing due to very strange issue, for which I have asked help of |
This patch adjust cffi definitions and also all associated functions based on libyang v4.2.2. Signed-off-by: Stefan Gula <[email protected]>
d6faad4 to
8c24656
Compare
|
Hi, |
Unfortunately this is little bit outside of my expertise as I don't use this feature of libyang, I was just trying to fix the tests. The issue was actually taken care of directly by
My approach was to minimize the number of backward incompatible changes to bare minimum. But adding one more is not that big of deal. So I will leave it up to maintainers of this repo to decide |
This PR introduces several commits needed for proper functionality with libyang v4.1.0