Encode from struct #78
Conversation
Compiles, but tests are not done yet Also fixed Github workflows config
This only tests strings but makes sure that the general mechanism works, at least for minimally what we need. More tests should be added but this covers our use case.
|
As a note, this does not include arrays and a few other things because it isn;t clear how this is supposed to work. Feel free to merge and document the omissions or add the those small parts. |
|
(or suggest how it is supposed to work) |
magiconair
left a comment
magiconair
left a comment
There was a problem hiding this comment.
Thank you for the PR. Can you provide a motivation for this change? What problem are you trying to solve?
There are some left-overs from your fork and the GitHub Actions workflow needs to be reverted. GOPATH is deprecated.
There need to be more tests and I think the tests should render a struct to a properties file. See properties_test.go.
The generated properties should be compatible with what Decode()expects. You can cover that in the test as well.
| @@ -0,0 +1,12 @@ | |||
| # Delivery Hero helm-charts Security | |||
| } | ||
| } | ||
| return nil | ||
| // TODO: Implement arrays and maps here. This is not needed in the |
There was a problem hiding this comment.
What is the problem you need to solve? Naming the keys?
The generated properties file needs to be compatible with what Decode() expects.
There was a problem hiding this comment.
The specific problem here is that it wasn't immediately clear what the output should look like. Comma separated lists for arrays? What for maps?
| return tm | ||
| } | ||
|
|
||
| func testEncode(t *testing.T) { |
There was a problem hiding this comment.
I think a set of tests which compares the struct with the generated properties file content would be better. Test that you can decode them into the same struct with Decode().
It should probably live in properties_test.go.
| P := NewProperties() | ||
| V := S{"Foo", "bar", "baz"} | ||
| err := P.Encode(V) | ||
| if (err != nil) { |
There was a problem hiding this comment.
You don't need parentheses here.
|
Ok I will fix the changes noted above. The motivation is that I want to create properties files (for Liquibase) using this module and the functionality isn't present. Using the module with its current functionality provides better testability (because I can do read/write roundtrips) and it provides a feature which was previously requested by others. Otherwise I have to inline the serialization functions in the other code and that doesn't feel very right. |
einhverfr
force-pushed
the
encode
branch
2 times, most recently
from
2ef8ae7 to
dc133df
Compare
| // Encode adds information to the Properties object based on the exported | ||
| // fields of the struct. | ||
| // | ||
| // In this first version, arrays and maps are unsupported. Neither are times |
There was a problem hiding this comment.
This is no longer correct, is it?
2 participants
This provides some basic encoding possibilities