robshakir / pyangbind

A plugin for pyang that creates Python bindings for a YANG model.
Other
204 stars 121 forks source link

Fix _is_config attribute for non-config containers #222

Closed nicolasmorini closed 11 months ago

nicolasmorini commented 6 years ago

When a YANG definition of a container has the statement "config false;", the Python bindings generated for this container should have "is_config=False" in its init method, but it was not the case.

Containers nested in non-config containers correctly inherited the _is_config attribute from its parent.

Add two test cases to tests/config-false.

googlebot commented 6 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers
nicolasmorini commented 6 years ago

I signed it!

googlebot commented 6 years ago

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

nicolasmorini commented 6 years ago

I registered my work email address!

googlebot commented 6 years ago

CLAs look good, thanks!

codecov[bot] commented 6 years ago

Codecov Report

Merging #222 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #222   +/-   ##
=======================================
  Coverage   71.37%   71.37%           
=======================================
  Files           7        7           
  Lines        1806     1806           
  Branches      484      484           
=======================================
  Hits         1289     1289           
  Misses        382      382           
  Partials      135      135

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 9aaf533...9a7a042. Read the comment docs.

tarkatronic commented 6 years ago

Aside from the one comment, the code LGTM. I'll wait on @robshakir for approval of the behavior. 👍

JoseIgnacioTamayo commented 1 year ago

Hi,

Could you please rebase to a recent versions of pyangbind ?

Thanks.

nicolasmorini commented 1 year ago

Hi @JoseIgnacioTamayo!

Unfortunately I have moved on from YANG/Netconf and I lost most of the context for this pull request.

Feel free to take over it if you find it useful, or to just close it altogether.

Best regards, Nicolas

fperrin commented 1 year ago

I can confirm this issue is still present, and at $work we have been running with Nicolas' change for a while (thank you @nicolasmorini !). Opened #323 after rebasing over newest master.

JoseIgnacioTamayo commented 11 months ago

Fixed in https://github.com/robshakir/pyangbind/pull/323