-
Notifications
You must be signed in to change notification settings - Fork 114
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
[pstl-offload] PSTL Offload files cleanup #1752
Conversation
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.
This looks good to me. I do recommend removing lines instead of commenting them out in the CMakeLists.txt files.
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.
LGTM with removal of commented line
28cb459
to
134c135
Compare
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.
LGTM!
While I am approving this PR, I strongly prefer that either @danhoeflinger or @dmitriy-sobolev (or both) would review infrastructure modifications
I will try to take a look today. |
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.
The changes in the CI look good to me.
The changes in the CMake look to me as well after a quick look. There are still many PSTL offload specific things in oneDPL CMake files, but we agreed to leave them to temporarily preserve opportunity to test PSTL offload through oneDPL's test framework.
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.
LGTM from a cmake and ci perspective. I've not looked in detail at the other changes.
786ccb1
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.
LGTM
No description provided.