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

Fix problem where a Choice statement contains Choice statements (yang… #222

Closed

Conversation

johannemitzcisco
Copy link

Fix problem where a Choice statement contains Choice statements (yang1.1) and Issue #75 (augments in Uses)

Signed-off-by: Johan Nemitz [email protected]

… 1.1) and Issue openconfig#75 (augments in Uses)

Signed-off-by: Johan Nemitz <[email protected]>
@wenovus
Copy link
Collaborator

wenovus commented Mar 3, 2022

Copying my comment from #221:

There was an attempt to do this before (#94) but since this is a breaking change there was a discussion on how best to handle it: #51 (comment)

Given we have just released v1.0.0 it seems appropriate to not violate backwards compatibility and simply introduce the Augments field instead as was the latest intention in the link above. This field is not being currently used by the goyang library but evidently users are using it.

@robshakir unfortunately I didn't record any decisions we may have made on this when we went through all the pending issues in preparation for v1.0.0. Let us know if you have objections to introducing an Augments field to support YANG1.1.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.008% when pulling aed5609 on johannemitzcisco:no-such-module into de640d0 on openconfig:master.

@robshakir
Copy link
Contributor

I'm OK with adding an Augments statement here, and gradually phasing out Augment if we think that this is something that would be reasonable to do. I suggest we populate both fields in the intermediate phase.

The Choice change looks backwards compatible.

Let's ensure that we have test coverage here before we merge it please :-)

@wenovus
Copy link
Collaborator

wenovus commented Mar 3, 2022

I think Paul's comment here is what I'm supporting we implement: #51 (comment)

Johan let us know if you need any help with this.

@johannemitzcisco johannemitzcisco deleted the no-such-module branch March 3, 2022 21:19
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.

4 participants