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

pkg: test that "empty" RepoConfig marshal to "empty" json #303

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Dec 5, 2023

This is a regression test for an issue found during the review of PR#284 [0]. The issue was that

	ModuleHotfixes *bool    `json:"module_hotfixes"`

lacked the required omitempty which may cause issues with dnfjson.

As a quick and easy way to prevent this in the future this commit adds a small test that ensures that we notice any missing omitempty.

Note that the rpmmd part is not strictly needed but it feels more correct to do the check in both places that use a repoConfig.

[0] #284 (comment)

This is a regression test for an issue found during the review
of PR#284 [0]. The issue was that
```
	ModuleHotfixes *bool    `json:"module_hotfixes"`
```
lacked the required `omitempty` which may cause issues
with dnfjson.

As a quick and easy way to prevent this in the future
this commit adds a small test that ensures that we notice
any missing `omitempty`.

Note that the `rpmmd` part is not strictly needed but it
feels more correct to do the check in both places that
use a repoConfig.

[0] osbuild#284 (comment)
Copy link
Contributor

@bcl bcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@bcl bcl added this pull request to the merge queue Dec 6, 2023
Merged via the queue into osbuild:main with commit 8a6dfee Dec 6, 2023
9 checks passed
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.

2 participants