Skip to content

Conversation

@RyanUnderhill
Copy link
Contributor

The current prefix was too long and making the API functions hard to read.

This changes to the simple prefix 'Ort' for 'Onnx RunTime' and uses it for all macros and function names in the public headers. This doesn't replace all occurrences of OnnxRuntime\ONNXRuntime\ONNXRUNTIME_ in the code. That can be done next if this change is approved.

@RyanUnderhill RyanUnderhill requested a review from a team as a code owner December 14, 2018 01:52
@snnn
Copy link
Contributor

snnn commented Dec 14, 2018

'Ort'
'Onnx'
'Orz'

:-)

virtual MLDataType GetElementType() const {
// should never reach here.
ONNXRUNTIME_NOT_IMPLEMENTED(__FUNCTION__, " is not implemented");
ORT_NOT_IMPLEMENTED(__FUNCTION__, " is not implemented");
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, having some macros that have ONNXRUNTIME_ and some ORT_ doesn't look very clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll make them all ORT_

@pranavsharma pranavsharma requested a review from a team December 14, 2018 09:38
Copy link
Contributor

@yuanbyu yuanbyu left a comment

Choose a reason for hiding this comment

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

LGTM.

@RyanUnderhill RyanUnderhill merged commit 11b369a into master Dec 14, 2018
@RyanUnderhill RyanUnderhill deleted the ryanunderhill/c_api_3 branch December 14, 2018 22:56
souptc pushed a commit that referenced this pull request Dec 17, 2018
Will enable more node tests(notable, those converted from pytorch) in the future.

Related work items: #164, #175
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants