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

pull request for issue 78 #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

flube2
Copy link

@flube2 flube2 commented Aug 29, 2017

For a software design class we were to write a patch, whether it was accepted or not. Due to this, we implemented the code for this issue before submitting a pull request. If changes are needed, let me know.

StrmanTests.java - Test cases created and added to file
NumberFormatOptions.java - Created for implementing formatting options
Strman.java - formatNumber() implemented

The only huge deviation so far is that the desired argument passed to the formatNumber() function was listed on shekargulati/strman-java as type "long". This will not work with decimal numbers. Changing it to "double" solves all problems that we had encountered.

@codecov-io
Copy link

Codecov Report

Merging #93 into master will decrease coverage by 0.07%.
The diff coverage is 93.54%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #93      +/-   ##
============================================
- Coverage     98.87%   98.79%   -0.08%     
- Complexity      202      222      +20     
============================================
  Files             3        4       +1     
  Lines          4183     4245      +62     
  Branches         55       66      +11     
============================================
+ Hits           4136     4194      +58     
  Misses           24       24              
- Partials         23       27       +4
Impacted Files Coverage Δ Complexity Δ
src/main/java/strman/NumberFormatOptions.java 100% <100%> (ø) 5 <5> (?)
src/main/java/strman/Strman.java 87.4% <91.83%> (+0.63%) 215 <15> (+15) ⬆️

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 7b552eb...28546ed. Read the comment docs.

@shekhargulati
Copy link
Owner

@flube2 I have started review of this PR. Will try to do it by EOD

@shekhargulati shekhargulati self-requested a review September 1, 2017 05:03
@flube2
Copy link
Author

flube2 commented Sep 21, 2017

Ok let me know if there are any changes you would like me to make

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