search

freeconf / yang

Standards-based management for Golang microservices
Apache License 2.0
38 stars 15 forks source link

Copy union subtypes when mixing them in. #97

Closed HRogge closed 9 months ago

HRogge commented 9 months ago

Subtypes of unions are pointers. Copy the content of them in the mixin() function to prevent global redefinition to list types for named types.

dhubler commented 9 months ago

Reviewed the PR and looks good. Copying the union type in makes a lot of sense.

Only thing I would have changed was unit tests, I generally create a YANG file in parser/testdata and dump the parse tree to a file for comparison. Turns out there already was a case that tested union + typedefs + leaf-lists and it started "failing" with the PR only because the test assertion was incorrect.

To see test "failure" run

cd parser
go test -run TestParseSamples

and you will see a diff. To update the assertion, you could edit the json file, but instead just run:

go test -run TestParseSamples -update

and now the test passed and the test assertion file captures the new fixed, results. Please add that update to this PR.

Thanks a lot!

HRogge commented 9 months ago

I really hope we don't have another of these graph like dependencies... I still have some trouble exactly understanding what the combination of YACC parser and compile.go are doing.