CR feedback by smk2007 · Pull Request #2970 · microsoft/onnxruntime

smk2007 · 2020-02-03T23:50:46Z

User/sheilk/windowsai merge cr feedback

cmake/CMakeLists.txt

fdwr · 2020-02-03T23:54:14Z

include/onnxruntime/core/session/onnxruntime_c_api.h

+	 * This api augments OrtTypeInfo to return denotations on the type.
+	 * This is used by WinML to determine if an input/output is intended to be an Image or a Tensor.
+    */
+  OrtStatus*(ORT_API_CALL* GetDenotationFromTypeInfo)(_In_ const OrtTypeInfo*, _Out_ const char** const denotation, _Out_ size_t* len)NO_EXCEPTION;


[nit] Add some spaces for readability before and after the function? Or is stupid clang-format messing things up?

OrtStatus* (...)(...) NO_EXCEPTION; #Resolved

Ah, seems you just cut&paste this from elsewhere. Non-blocking minor. #Resolved

same formatting as other places in this file!

In reply to: 374407768 [](ancestors = 374407768)

yep

In reply to: 374408058 [](ancestors = 374408058)

snnn

Thanks

docs/Privacy.md

smk2007 · 2020-02-03T23:56:55Z

YW!

In reply to: 352644894 [](ancestors = 352644894)

ryanlai2 · 2020-02-04T00:08:47Z

cmake/precompiled_header.cmake

@@ -1,3 +1,6 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.


Oh copy/paste error.

In reply to: 374411856 [](ancestors = 374411856)

Same thing for the other .cmake files and also in CMakeLists.txt?

In reply to: 374412145 [](ancestors = 374412145,374411856)

hmm... seems like All rights reserved is more common in the code base... switching to that.

In reply to: 374412543 [](ancestors = 374412543,374412145,374411856)

include/onnxruntime/core/providers/winml/winml_provider_factory.h

zhangxiang1993 · 2020-02-04T00:20:09Z

winml/adapter/winml_adapter_dml.cpp

@@ -1,5 +1,6 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.


revert? #Resolved

zhangxiang1993 · 2020-02-04T00:20:23Z

winml/adapter/winml_adapter_environment.cpp

@@ -1,5 +1,6 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.


revert?
#Resolved

zhangxiang1993 · 2020-02-04T00:20:41Z

winml/adapter/winml_adapter_execution_provider.cpp

@@ -1,5 +1,6 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.


same. #Resolved

zhangxiang1993 · 2020-02-04T00:24:28Z

cmake/precompiled_header.cmake

@@ -1,3 +1,6 @@
+# Copyright (c) Microsoft Corporation.


tools/ci_build/github/azure-pipelines/win-gpu-ci-pipeline.yml

zhangxiang1993 · 2020-02-04T00:29:57Z

 python $(Build.SourcesDirectory)\tools\ci_build\build.py --config $(BuildConfig) --build_dir $(Build.BinariesDirectory) --skip_submodule_sync --build_shared_lib --test --cmake_generator "Visual Studio 16 2019" --msvc_toolset 14.16 --build_wheel --use_featurizers --use_dnnl --build_shared_lib --enable_onnx_tests --use_dml --use_cuda --cuda_version=10.0 --cuda_home="C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v10.0" --cudnn_home="C:\local\cudnn-10.0-windows10-x64-v7.6.5.32\cuda" --cmake_extra_defines CMAKE_SYSTEM_VERSION=10.0.18362.0

nit: two lines #Resolved

Refers to: tools/ci_build/github/azure-pipelines/win-gpu-ci-pipeline.yml:119 in 784cefa. [](commit_id = 784cefa, deletion_comment = False)

zhangxiang1993

CR feedback

26bbbef

smk2007 requested review from fdwr, martinb35, orilevari, ryanlai2, tiagoshibata, walrusmcd and zhangxiang1993 February 3, 2020 23:50

smk2007 requested a review from a team as a code owner February 3, 2020 23:50

ryanlai2 reviewed Feb 3, 2020

View reviewed changes

cmake/CMakeLists.txt Show resolved Hide resolved

fdwr reviewed Feb 3, 2020

View reviewed changes

snnn approved these changes Feb 3, 2020

View reviewed changes

tiagoshibata reviewed Feb 3, 2020

View reviewed changes

docs/Privacy.md Outdated Show resolved Hide resolved

fix weird formatting on privacy readme

7ed9dba

ryanlai2 reviewed Feb 4, 2020

View reviewed changes

zhangxiang1993 reviewed Feb 4, 2020

View reviewed changes

include/onnxruntime/core/providers/winml/winml_provider_factory.h Show resolved Hide resolved

zhangxiang1993 reviewed Feb 4, 2020

View reviewed changes

Sheil Kumar added 2 commits February 3, 2020 16:20

remove extra space in comment

784cefa

zhangxiang1993 reviewed Feb 4, 2020

View reviewed changes

tools/ci_build/github/azure-pipelines/win-gpu-ci-pipeline.yml Show resolved Hide resolved

remove extra whitespace

b7a4cc1

zhangxiang1993 approved these changes Feb 4, 2020

View reviewed changes

smk2007 merged commit 3f0b635 into windowsai Feb 4, 2020

smk2007 deleted the user/sheilk/windowsai-merge-cr-feedback branch February 4, 2020 00:36

		@@ -1,3 +1,6 @@
		# Copyright (c) Microsoft Corporation. All rights reserved.

		@@ -1,5 +1,6 @@
		// Copyright (c) Microsoft Corporation. All rights reserved.

Conversation

smk2007 commented Feb 3, 2020

Uh oh!

Uh oh!

fdwr Feb 3, 2020 • edited by smk2007 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fdwr Feb 3, 2020 • edited by smk2007 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smk2007 Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

smk2007 Feb 3, 2020

Choose a reason for hiding this comment

Uh oh!

snnn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smk2007 commented Feb 3, 2020

Uh oh!

ryanlai2 Feb 4, 2020 • edited by smk2007 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smk2007 Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanlai2 Feb 4, 2020

Choose a reason for hiding this comment

Uh oh!

smk2007 Feb 4, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zhangxiang1993 Feb 4, 2020 • edited by smk2007 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangxiang1993 Feb 4, 2020 • edited by smk2007 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangxiang1993 Feb 4, 2020 • edited by smk2007 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangxiang1993 Feb 4, 2020 • edited by smk2007 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zhangxiang1993 commented Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhangxiang1993 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fdwr Feb 3, 2020 •

edited by smk2007

Loading

fdwr Feb 3, 2020 •

edited by smk2007

Loading

ryanlai2 Feb 4, 2020 •

edited by smk2007

Loading

smk2007 Feb 4, 2020 •

edited

Loading

zhangxiang1993 Feb 4, 2020 •

edited by smk2007

Loading

zhangxiang1993 Feb 4, 2020 •

edited by smk2007

Loading

zhangxiang1993 Feb 4, 2020 •

edited by smk2007

Loading

zhangxiang1993 Feb 4, 2020 •

edited by smk2007

Loading

zhangxiang1993 commented Feb 4, 2020 •

edited

Loading