-
Notifications
You must be signed in to change notification settings - Fork 56
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
fix Incorrect DiffInMonths calculation #59 #60
fix Incorrect DiffInMonths calculation #59 #60
Conversation
…0, so diffInMonths can work properly
@rendyreivaldy25 I know it's not your fault, but before merging this we need to fix the CI/CD (current Travis) first. |
glad to help. please contact me if my code has a problem. |
@rendyreivaldy25 can you rebase with master? |
…econd carbon instance is less than the first
@tlfbrito I am so sorry. I was so busy until I forgot to check this issue. I made some changes again because in my last works, different bugs appeared. Right now it should be running okay. Please kindly check if there are other bugs. Thanks |
if carb.Timestamp() < c.Timestamp() { | ||
remainingTime = int(c.DiffInHours(carb, true)) | ||
totalHours = carb.DaysInMonth() * hoursPerDay | ||
} else { | ||
remainingTime = int(carb.DiffInHours(c, true)) | ||
totalHours = c.DaysInMonth() * hoursPerDay | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you isolate this in a function? and return instead of using if-else statement?
if carb.Timestamp() < c.Timestamp() { | |
remainingTime = int(c.DiffInHours(carb, true)) | |
totalHours = carb.DaysInMonth() * hoursPerDay | |
} else { | |
remainingTime = int(carb.DiffInHours(c, true)) | |
totalHours = c.DaysInMonth() * hoursPerDay | |
} | |
remainingTime, totalHours = calculateDiffInTimeAndHours(c, carb) | |
// ... | |
func calculateDiffInTimeAndHours(c, carb * Carbon) (int64, int64) { | |
if carb.Timestamp() < c.Timestamp() { | |
return int(c.DiffInHours(carb, true)), carb.DaysInMonth() * hoursPerDay | |
} | |
return int(carb.DiffInHours(c, true)), c.DaysInMonth() * hoursPerDay | |
} | |
} |
hi @rendyreivaldy25, just left you a suggestion! Let me know what you think :) |
Sorry for slow response, of course, I will fox that later. I will update to you soon |
Codecov Report
@@ Coverage Diff @@
## master #60 +/- ##
==========================================
+ Coverage 89.11% 90.17% +1.05%
==========================================
Files 3 3
Lines 634 641 +7
==========================================
+ Hits 565 578 +13
+ Misses 44 39 -5
+ Partials 25 24 -1
Continue to review full report at Codecov.
|
Due to lack of activity - Fixing this on #74 |
added condition check if diffInMonths > 0, so diffInMonths can work properly.
issue #59