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

fix: RequiredIfNoDef with nested structs #229

Merged
merged 2 commits into from Jul 13, 2022
Merged

fix: RequiredIfNoDef with nested structs #229

merged 2 commits into from Jul 13, 2022

Conversation

HarmlessEvil
Copy link
Contributor

Fix summary

I think, we simply need to check not if the key (which is prefix + key) is empty, but if the key itself. For this, I introduced variable ownKey. All other usages of key remained the same. All tests have passed and I added my own test to cover this bug.

Copy link
Owner

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

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

needs gofumpf, other than that lgtm!

thanks for the PR!

@codecov
Copy link

codecov bot commented Jul 12, 2022

Codecov Report

Merging #229 (1cbc90f) into main (a5d7cf5) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #229   +/-   ##
=======================================
  Coverage   98.84%   98.84%           
=======================================
  Files           2        2           
  Lines         345      345           
=======================================
  Hits          341      341           
  Misses          4        4           
Impacted Files Coverage Δ
env.go 98.81% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5d7cf5...1cbc90f. Read the comment docs.

@caarlos0 caarlos0 changed the title Fix bug https://github.com/caarlos0/env/issues/228 fix: RequiredIfNoDef with nested structs Jul 12, 2022
@caarlos0
Copy link
Owner

closes #228

@HarmlessEvil
Copy link
Contributor Author

needs gofumpt

Should be done :)

@caarlos0 caarlos0 merged commit 439dbad into caarlos0:main Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants