Skip to content

Fixes #183 - #185

Merged
nicklucius merged 8 commits into
masterfrom
issue183
Oct 23, 2019
Merged

Fixes #183#185
nicklucius merged 8 commits into
masterfrom
issue183

Conversation

@nicklucius

Copy link
Copy Markdown
Contributor

Due to a change in an upstream data source, test code had broken. This fixes that code.

@nicklucius
nicklucius requested a review from geneorama October 23, 2019 15:17
@nicklucius nicklucius self-assigned this Oct 23, 2019
@coveralls

coveralls commented Oct 23, 2019

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.5%) to 83.784% when pulling 527a34e on issue183 into 8befd4f on master.

@geneorama

Copy link
Copy Markdown
Member

I'm more inclined to remove the column count tests unless we can comment why they're relevant.

I would also like to add some additional comments about what these tests are doing.

@nicklucius

Copy link
Copy Markdown
Contributor Author

@geneorama I think we will have a better answer about the column questions when we resolve #184. The test description is "Warn instead of fail if X-SODA2-* headers are missing" and we could add to that after resolving #184 as well.

If you want to remove the column checks while #184 is open I don't see an issue with that.

@geneorama

Copy link
Copy Markdown
Member

@nicklucius can you review please?

Made changes based on our conversation. I don't think the checks for data frame or column numbers were related to the original issue. The column number check was the part that was failing, so to me the cleanest way is to adjust the test.

@geneorama

Copy link
Copy Markdown
Member

Based on the verbal discussion, good idea to add the message relating to issue 118

@nicklucius

Copy link
Copy Markdown
Contributor Author

Looks good @geneorama

@nicklucius
nicklucius merged commit 7791891 into master Oct 23, 2019
@geneorama

Copy link
Copy Markdown
Member

@nicklucius thanks. Glad we could get this knocked out.

@geneorama
geneorama deleted the issue183 branch September 1, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants