Skip to content

Prevent ParseSamples errors from being hidden - #12

Open
carbocation wants to merge 1 commit into
brentp:masterfrom
carbocation:feature/parsing-errors
Open

carbocation wants to merge 1 commit into
brentp:masterfrom
carbocation:feature/parsing-errors

Conversation

@carbocation

Copy link
Copy Markdown

The header.ParseSamples method overwrites the errors slice with every new sample. So, if the very last sample has no errors, then header.ParseSamples reports that there was no error.

This commit causes header.ParseSamples to accumulate all errors in sample parsing, permitting it to indicate if any sample was incorrectly parsed, rather than the current behavior which only tells you whether or not the last sample was incorrectly parsed.

(Note: This came to my attention while troubleshooting what I think is an issue with my io.Reader wrapper from #11 , which may be nulling out data as you had pointed out.)

field. So, if the last sample did not have errors, then no errors
would be reported. This corrects that by accumulating all errors.
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 65.763% when pulling db84224 on carbocation:feature/parsing-errors into 950a2c3 on brentp:master.

5 similar comments
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 65.763% when pulling db84224 on carbocation:feature/parsing-errors into 950a2c3 on brentp:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 65.763% when pulling db84224 on carbocation:feature/parsing-errors into 950a2c3 on brentp:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 65.763% when pulling db84224 on carbocation:feature/parsing-errors into 950a2c3 on brentp:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 65.763% when pulling db84224 on carbocation:feature/parsing-errors into 950a2c3 on brentp:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 65.763% when pulling db84224 on carbocation:feature/parsing-errors into 950a2c3 on brentp:master.

@brentp

brentp commented Mar 15, 2019

Copy link
Copy Markdown
Owner

this seems reasonable. can you resolve the conflicts? or has this been fixed in a more recent version?

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.

3 participants