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.

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

Moved ngraph::Node to ov namespace #7240

Merged
merged 14 commits into from
Aug 30, 2021

Conversation

ilyachur
Copy link
Contributor

No description provided.

@ilyachur ilyachur added the category: Core OpenVINO Core (aka ngraph) label Aug 25, 2021
@ilyachur ilyachur added this to the 2022.1 milestone Aug 25, 2021
@ilyachur ilyachur requested a review from a team as a code owner August 25, 2021 12:30
@ilyachur ilyachur requested a review from a team August 25, 2021 13:19
@ilyachur ilyachur requested a review from a team August 25, 2021 14:51
Comment on lines +40 to +41
/// TODO: Make a plan to deprecate this.
Output(const std::shared_ptr<Node>& node, size_t index);
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this and other TODO comments are copied from the original file, but maybe we have a good moment to review and update such info to have a clear situation about deprecation plans.
I don't mean it's needed in this PR but still related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will do it in the separate PR. it affects a lot of places

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any reason to remove this c-tor as in most cases we are working with std::shared_ptr<Node> but not with Node * so it is more convenient and doesn't have any side-effects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I already said, I tried to deprecate this constructor, but it affects several places. I don't want to mix these changes in this PR.

@ilyachur ilyachur requested review from a team as code owners August 26, 2021 05:28
Comment on lines +40 to +41
/// TODO: Make a plan to deprecate this.
Output(const std::shared_ptr<Node>& node, size_t index);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any reason to remove this c-tor as in most cases we are working with std::shared_ptr<Node> but not with Node * so it is more convenient and doesn't have any side-effects.

@ilyachur ilyachur merged commit 6cdd76f into openvinotoolkit:master Aug 30, 2021
@ilyachur ilyachur deleted the move_node_to_ov branch August 30, 2021 03:20
akuporos pushed a commit to akuporos/openvino that referenced this pull request Sep 29, 2021
* Moved ngraph::Node to ov namespace

* Fixed code style

* Fixed VPU

* Fixed GNA

* Fixed tests

* Added aliases for backward compatibility

* Fix clDNN

* Try to fix build

* Fixed comment

* Renamed RTTI macros
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Core OpenVINO Core (aka ngraph)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants