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

Added safety check len == 0 #109

Merged
merged 2 commits into from
Oct 29, 2019
Merged

Added safety check len == 0 #109

merged 2 commits into from
Oct 29, 2019

Conversation

SloCompTech
Copy link
Contributor

Fixing issue #69, compiled with success, not tested. Would anyone test that it is working ?

P. S. Please check if my identation is correct, I set it to 4 spaces like it said in HACKING.md.

Copy link
Member

@darconeous darconeous left a comment

Choose a reason for hiding this comment

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

Looks good, but I know @smortex loves unit tests. I personally wouldn't hold this change back for lack of one, but let's see what he thinks.

@smortex smortex merged commit d13eca5 into nfc-tools:master Oct 29, 2019
@smortex
Copy link
Contributor

smortex commented Oct 29, 2019

Oh yes, that was open for a long time. Never mind, the unit tests are not critical for this and the behavior was wrong. Thanks!

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.

None yet

3 participants