-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Conversation
This will be merged once we sign off #3065 |
@guihao-liang - I'm really not understanding the purpose of this change from your description. Also it looks like a lot of the changes here are just formatting changes. If you're making a large number of formatting changes, it would be helpful to reviewers to do those as a separate commit. |
Yeah, I marked those places that need to be reviewed. Well, the title explains, I set cmake policy 0045 0046 from OLD to NEW, which means it will error out when a target doesn't exist. |
Passed internal pipeline. Let's merge. |
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.
Just rework the formatting mess-ups and looks fine.
code review purpose for #3065;
basically, it checks whether the dependencies exist or not. Otherwise, CMake will complain about the target
""
is not found.