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

incrementing and decrementing the data so that 0s display now as 1s #139

Merged
merged 1 commit into from
Mar 21, 2017

Conversation

samhstn
Copy link
Contributor

@samhstn samhstn commented Mar 21, 2017

Fixes #138

Copy link

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

@Shouston3 LGTM. 👀 👍

@codecov
Copy link

codecov bot commented Mar 21, 2017

Codecov Report

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

@@            Coverage Diff             @@
##           master     #139      +/-   ##
==========================================
+ Coverage   99.32%   99.32%   +<.01%     
==========================================
  Files          28       28              
  Lines         296      298       +2     
==========================================
+ Hits          294      296       +2     
  Misses          2        2
Impacted Files Coverage Δ
web/controllers/survey_controller.ex 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8862aaa...f793631. Read the comment docs.

@nelsonic
Copy link

@Shouston3 what would it take to add a test to cover these last two lines of code? 🤔

@samhstn
Copy link
Contributor Author

samhstn commented Mar 21, 2017

We just need to abstract out the function into a separate helper which manages the url to use and get our coverage to ignore this file.

We should be tying up these loose ends today. Added as a separate issue #140

@jackcarlisle jackcarlisle merged commit 22e096e into master Mar 21, 2017
@jackcarlisle jackcarlisle deleted the inc-and-dec-data branch March 21, 2017 11:26
@nelsonic
Copy link

nelsonic commented Mar 21, 2017

@Shouston3 good plan! 👍
Would you mind investigating how to setup a pre-commit hook
to check coverage on localhost before a commit is allowed?
see: dwyl/learn-pre-commit#6
(doesn't have to be today but sometime would be awesome!) ✨

@samhstn
Copy link
Contributor Author

samhstn commented Mar 21, 2017

It may not have worked well for us because of the local coverage issue here: dwyl/learn-phoenix#55

Currently, our coverage is never 100% locally

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