search

freeconf / yang

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

fix copy on append bug in reflection #98

Closed sibuk-harabudjasim closed 8 months ago

sibuk-harabudjasim commented 9 months ago

Hi, Found a problem during working with lists in YANG. Seems that adding new element to tree, keeps reference to original created element, not a copy that a put to slice during reflect.Append(). This code fixes that. Also adds some support for arrays, but feel free to remove this. Also, please, point if you need some additional tests or test coverage to that change.

dhubler commented 9 months ago

thanks for the PR. Adding array support is good. I would want a unit test because certainly a tricky issue and fix. I would add a section to reflect_test.go:TestReflect2Write but up to you. Also unit test for array would be prudent as well.

Also, consider looking at nodeutil.Node for reflection based operations. It is much more powerful in a lot of ways and I hope someday to obsolete Reflect.

sibuk-harabudjasim commented 9 months ago

Okay, I'll add test and ping you when PR will be ready.

sibuk-harabudjasim commented 9 months ago

I've added testcase with slice of structs and cleaned up code a bit. Also I dropped cases made for arrays, since there's lot more work is needed to handle limited number of items. Please check if it's OK for you.

dhubler commented 9 months ago

awesome, i'll take a look at this later today.