Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Windows cmake flags cleanup #16013

Merged
merged 1 commit into from
Aug 28, 2019
Merged

Windows cmake flags cleanup #16013

merged 1 commit into from
Aug 28, 2019

Conversation

larroy
Copy link
Contributor

@larroy larroy commented Aug 27, 2019

Description

See tittle. Doesn't change any behaviour.
Flags were set to 0 and 1 instead of ON OFF
Removed USE_PROFILER
#15595

@larroy
Copy link
Contributor Author

larroy commented Aug 27, 2019

@mxnet-label-bot add [Build]

@larroy
Copy link
Contributor Author

larroy commented Aug 27, 2019

@ChaiBapchya

Copy link
Contributor

@ChaiBapchya ChaiBapchya left a comment

Choose a reason for hiding this comment

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

LGTM!

@yuxihu
Copy link
Member

yuxihu commented Aug 28, 2019

The changes look straightforward. Two questisons:

  1. ON/OFF is the preferred/recommended way of setting cmake flags? 0/1 also worked, right?
  2. Why removing USE_PROFILER? Is it ON in other platforms? I think we have profiler related tests. Would it be affected?

@larroy
Copy link
Contributor Author

larroy commented Aug 28, 2019

USE_PROFILER was deprecated by @ChaiBapchya ON OFF is preferred as it was requested to have multi value flag for the openmp flag in a different PR.

Copy link
Member

@yuxihu yuxihu left a comment

Choose a reason for hiding this comment

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

LGTM.

@yuxihu yuxihu merged commit b2c0cbc into apache:master Aug 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants