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

jsonsaver: omit empty lists rather than null #106

Merged
merged 3 commits into from Mar 13, 2022
Merged

jsonsaver: omit empty lists rather than null #106

merged 3 commits into from Mar 13, 2022

Conversation

CatalinStratu
Copy link
Contributor

@CatalinStratu CatalinStratu commented Mar 12, 2022

Fixes #103

Signed-off-by: Catalin Stratu <catalinstratu45@gmail.com>
Signed-off-by: Catalin Stratu <catalinstratu45@gmail.com>
@swinslow
Copy link
Member

Hi @CatalinStratu, thanks for this! Looks like there's one test which is failing:

 === RUN   Test_renderSnippets2_2/success_empty
    save_snippets_test.go:142: renderSnippets2_2() = map[SPDXID:SPDXRef- ranges:[map[endPointer:map[offset:0 reference:SPDXRef-] startPointer:map[offset:0 reference:SPDXRef-]] map[endPointer:map[lineNumber:0 reference:SPDXRef-] startPointer:map[lineNumber:0 reference:SPDXRef-]]]], want map[ranges:[map[] map[]]]
--- FAIL: Test_renderSnippets2_2 (0.00s)
    --- PASS: Test_renderSnippets2_2/success (0.00s)
    --- FAIL: Test_renderSnippets2_2/success_empty (0.00s)
FAIL
coverage: 95.1% of statements
FAIL	github.com/spdx/tools-golang/jsonsaver/saver2v2	0.005s

Looks like it perhaps isn't correctly rendering the "empty" snippet case, can you take a look and see what's going on here? Once this is addressed I think the rest of it is good to merge. Thank you!

Signed-off-by: Catalin Stratu <catalinstratu45@gmail.com>
@CatalinStratu
Copy link
Contributor Author

Hi @CatalinStratu, thanks for this! Looks like there's one test which is failing:

 === RUN   Test_renderSnippets2_2/success_empty
    save_snippets_test.go:142: renderSnippets2_2() = map[SPDXID:SPDXRef- ranges:[map[endPointer:map[offset:0 reference:SPDXRef-] startPointer:map[offset:0 reference:SPDXRef-]] map[endPointer:map[lineNumber:0 reference:SPDXRef-] startPointer:map[lineNumber:0 reference:SPDXRef-]]]], want map[ranges:[map[] map[]]]
--- FAIL: Test_renderSnippets2_2 (0.00s)
    --- PASS: Test_renderSnippets2_2/success (0.00s)
    --- FAIL: Test_renderSnippets2_2/success_empty (0.00s)
FAIL
coverage: 95.1% of statements
FAIL	github.com/spdx/tools-golang/jsonsaver/saver2v2	0.005s

Looks like it perhaps isn't correctly rendering the "empty" snippet case, can you take a look and see what's going on here? Once this is addressed I think the rest of it is good to merge. Thank you!

Hi, thank you very much. I tried to fix the problem. Thanks!

@swinslow
Copy link
Member

Thanks @CatalinStratu! I see that reduces the "empty" test to just an empty document, which may be sufficient for the time being. I'm going to try out a couple more things with this PR, and if that looks good then I'll merge it shortly. Thanks for your quick updates on this one!

@swinslow
Copy link
Member

OK - as far as I can tell, this addresses the issues that were raised in #103. Thanks very much @CatalinStratu! Merging now.

@swinslow swinslow changed the title #103 jsonsaver: omit empty lists rather than null bug fixes jsonsaver: omit empty lists rather than null bug fixes Mar 13, 2022
@swinslow swinslow changed the title jsonsaver: omit empty lists rather than null bug fixes jsonsaver: omit empty lists rather than null Mar 13, 2022
@swinslow swinslow merged commit b38edad into spdx:main Mar 13, 2022
@swinslow swinslow added this to the 0.3.0 milestone Mar 13, 2022
@swinslow swinslow added the bug Something isn't working label Mar 13, 2022
@swinslow swinslow mentioned this pull request Mar 13, 2022
@CatalinStratu
Copy link
Contributor Author

OK - as far as I can tell, this addresses the issues that were raised in #103. Thanks very much @CatalinStratu! Merging now.

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jsonsaver: omit empty lists rather than null
2 participants