Skip to content

Bindings updates for 789 #801

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

Merged
merged 8 commits into from
Feb 19, 2021

Conversation

TheBlueMatt
Copy link
Collaborator

Slowly but surely, bindings updates for PRs are getting smaller and smaller...

Sadly, there's just not really a practical way to map a slice of
objects in our current bindings infrastructure - either we take
ownership of the underlying objects and move them into a Vec, or we
need to leave the original objects in place and have a list of
pointers to the Rust objects. Thus, the only practical mapping is
to create a slice of references using the pointers we have.
`from_c_conversion_container_new_var` should use var_access when
it wishes to access the variable being converted, not `var_name`,
but in a few cases it did not. Note that this has no impact on the
generated bindings as of this commit.
@codecov
Copy link

codecov bot commented Feb 19, 2021

Codecov Report

Merging #801 (6b52b0f) into main (de58bcf) will decrease coverage by 0.05%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #801      +/-   ##
==========================================
- Coverage   90.82%   90.77%   -0.06%     
==========================================
  Files          44       44              
  Lines       24282    24283       +1     
==========================================
- Hits        22054    22042      -12     
- Misses       2228     2241      +13     
Impacted Files Coverage Δ
lightning/src/ln/msgs.rs 88.62% <0.00%> (ø)
lightning/src/chain/keysinterface.rs 93.36% <100.00%> (ø)
lightning/src/ln/functional_tests.rs 96.95% <100.00%> (-0.23%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de58bcf...68811da. Read the comment docs.

Copy link
Contributor

@valentinewallace valentinewallace left a comment

Choose a reason for hiding this comment

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

LGTM! Seems like this got the bindings a bit closer to the underlying Rust code, which is nice

@TheBlueMatt
Copy link
Collaborator Author

Merging with one ACK since the full non-bindings diff is a trivial -3+3.

@TheBlueMatt TheBlueMatt merged commit ee68ffa into lightningdevkit:main Feb 19, 2021
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.

2 participants