Skip to content

Conversation

@devenbansod
Copy link
Member

Fix #189

Signed-off-by: Deven Bansod [email protected]

@codecov
Copy link

codecov bot commented Dec 21, 2018

Codecov Report

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

@@             Coverage Diff              @@
##             master     #212      +/-   ##
============================================
+ Coverage     99.75%   99.75%   +<.01%     
- Complexity     1736     1740       +4     
============================================
  Files            58       58              
  Lines          4147     4158      +11     
============================================
+ Hits           4137     4148      +11     
  Misses           10       10

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

LGTM :)

@devenbansod devenbansod force-pushed the fix/189 branch 2 times, most recently from 9b00223 to ca22db7 Compare December 21, 2018 14:04
@devenbansod
Copy link
Member Author

@williamdes any idea why the elseif line is being shown as not covered in the coverage report?

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

Thank you @devenbansod for the contribution !

@williamdes
Copy link
Member

@williamdes any idea why the elseif line is being shown as not covered in the coverage report?

Using the dropdown some tests seem to not cover the line, I do not know more 😕

@devenbansod
Copy link
Member Author

@williamdes I do believe (actually made sure code is being executed by putting a print statement in code while running) tests are touching the code. Not sure why it's not been caught here.

Anyway, I guess we can go ahead with merging this fix. I'll try to see if I can get it covered in some other PR maybe. Does it sound okay?

@williamdes
Copy link
Member

@devenbansod Yes, go ahead, okay for me 👍

@devenbansod
Copy link
Member Author

@williamdes it seems like it was just due to improper formatting. I moved the first condition of the elseif statement to previous line and now it's covering the entire diff. :-)

@devenbansod devenbansod merged commit e07403c into phpmyadmin:master Dec 21, 2018
@devenbansod devenbansod deleted the fix/189 branch December 21, 2018 18:12
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.

2 participants