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.

Sign up for GitHub

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

Jump to bottom

Replace CMath::ceil with std::ceil #4196

Merged
merged 1 commit into from Mar 23, 2018
Merged

Replace CMath::ceil with std::ceil #4196

merged 1 commit into from Mar 23, 2018

Conversation

syashakash
Copy link
Contributor

Refer #4186 .

  • Replaced CMtah::ceil with std::ceil in all files.

@syashakash syashakash force-pushed the replace-cmath-ceil branch 4 times, most recently from f60432b to 2d70378 Compare March 3, 2018 19:57
@karlnapf
Copy link
Member

karlnapf commented Mar 7, 2018

Could you pls send a PR where only the lines you changed are actually in the diff. There are some huge chunks of changes that are just due to formatting. The autoformater will only change your changes if you use it corectly

karlnapf
karlnapf reviewed Mar 7, 2018
View reviewed changes
src/shogun/structure/DynProg.cpp Outdated
if (std::ceil(penij->get_max_value()) > max_look_back)
{
SG_DEBUG(
"%d %d -> value: %f\n", ii, j,
Copy link
Member

Choose a reason for hiding this comment

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

these changes are unrelated, could you remove them?

karlnapf
karlnapf requested changes Mar 7, 2018
View reviewed changes
Copy link
Member

@karlnapf karlnapf left a comment

Choose a reason for hiding this comment

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

Looks good, remove the unrelated changes and we can merge. Thanks!

@syashakash syashakash force-pushed the replace-cmath-ceil branch 7 times, most recently from cd9c291 to c657e84 Compare March 18, 2018 21:25
@karlnapf
Copy link
Member

Looks good, thanks for the patch!

@karlnapf karlnapf merged commit 7605741 into shogun-toolbox:develop Mar 23, 2018
@syashakash syashakash deleted the replace-cmath-ceil branch March 23, 2018 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karlnapf karlnapf karlnapf requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@syashakash @karlnapf