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

Bump bindgen dependency #4

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

Conversation

Bucket-Bucket-Bucket
Copy link

Just a bindgen version update and a fix to one call that's been deprecated.

Recently did some OpenWrt building on Ubuntu 24.04 which failed due to a newer clang version being used. This update fixes it without any further issue.

@JKRhb JKRhb requested a review from pulsastrix May 24, 2024 10:33
@JKRhb JKRhb requested review from pulsastrix and removed request for pulsastrix October 10, 2024 15:59
Copy link
Member

@pulsastrix pulsastrix left a comment

Choose a reason for hiding this comment

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

Hi, and thanks for the contribution!

It seems like 5336811 includes the (unmerged) changes made in #3.
As the only other change in that commit is the addition of the Cargo.lock file (see my other comment), I'll probably merge #3 and then exclude that specific commit when merging this PR in order to preserve the proper commit history.

Aside from that and the other comment this PR looks good to me.

Just as a general disclaimer: Because we've not been actively working on a project targeting OpenWRT for quite some time1, this library has not been as well maintained as it probably should have been.

I intend to do some clean up and some smaller improvements before publishing a new release to crates.io (most notably #5 to allow for some basic automated testing).
However, because I am currently occupied with other stuff, this might take some time, and you might have to resort to using the git version of this crate until then.

Footnotes

  1. Our previous project that was using this library has not been maintained for a few years now.

Cargo.lock Outdated Show resolved Hide resolved
rust-uci/Cargo.toml Outdated Show resolved Hide resolved
@Bucket-Bucket-Bucket
Copy link
Author

Hi. Thank you for the feedback. Sorry for having the uci list changes included here. It wasn't intended but I had forgotten about this pr and just moved to using my fork as the dependency, since I needed the changes to be in.

@JKRhb JKRhb requested a review from pulsastrix October 26, 2024 09:16
@JKRhb
Copy link
Member

JKRhb commented Oct 26, 2024

Hi @Bucket-Bucket-Bucket, thank you for the update! Could you maybe remove 5336811 from this PR (e.g., using an interactive rebase)? I think then we could go forward with this one independently of the changes incorporated from #3.

@Bucket-Bucket-Bucket
Copy link
Author

Ended up just moving the uci list commits to a separate branch and dropping them from this, since they shouldn't have been here in the first place.

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

Successfully merging this pull request may close these issues.

3 participants