Skip to content

Conversation

@Kebniss
Copy link
Contributor

@Kebniss Kebniss commented May 3, 2018

This was a bug fix that was included in my last PR. This fix is important and would like to merge it even if the extra features do end up to make it to master :)

@Kebniss Kebniss requested a review from kmike May 3, 2018 06:40
@codecov
Copy link

codecov bot commented May 3, 2018

Codecov Report

Merging #59 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   81.01%   81.02%   +<.01%     
==========================================
  Files          40       40              
  Lines        2091     2092       +1     
==========================================
+ Hits         1694     1695       +1     
  Misses        397      397

@kmike
Copy link
Member

kmike commented May 4, 2018

Could you please add a test case?

@@ -0,0 +1,40 @@
from webstruct.features.global_features import _add_pattern_features
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind testing Pattern class instead? It should be an easy change, but this way we'd be testing public API

@kmike kmike merged commit 9fe8988 into master May 16, 2018
@kmike
Copy link
Member

kmike commented May 16, 2018

Thanks @Kebniss!

@kmike kmike deleted the fix-boolean-bug branch May 16, 2018 15:34
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