Skip to content

Add sample json #465

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

Closed
wants to merge 157 commits into from
Closed

Add sample json #465

wants to merge 157 commits into from

Conversation

JoeOster
Copy link
Contributor

@JoeOster JoeOster commented Apr 2, 2021

Description

Need to add sample.json to several jupyter notebooks, at one-time automation wasn't scanning, but now can. So needed to add

Type of change

  • Bug fix (non-breaking change which fixes an issue)

JoeOster and others added 30 commits August 11, 2020 14:47
 * Copyright (c) 2020 Intel Corporation
 *
 * This program and the accompanying materials are made available under the
 * terms of the The MIT License which is available at
 * https://opensource.org/licenses/MIT.
 *
 * SPDX-License-Identifier: MIT
 */
Updates per request of sranikonda
changing compiler name from "dpcpp-cl" to "dpcpp"
minor modifications to content, purpose and key implementation details.
aligned description with readme
reshuffled parts of the purpose and implementation details and abstracted a few key concepts into better summaries.
@JoeOster JoeOster requested a review from mdbtucker April 2, 2021 18:03
@mdbtucker
Copy link
Contributor

mdbtucker commented Apr 3, 2021

Joe, a few issues with this one:

  1. I believe the point of this review was to validate the sample.json that you're adding, but the validation seems to have failed.
  2. You seem to have accidentally added back the file you deleted in the last change I approved, please make sure you clean that up before merging. EDIT: I realize now you're DELETING this file in this change, not adding it back. But that deletion is part of another changelist so probably shouldn't be part of this one as well? Not sure if that's important, if you say it's not I'm happy to take your word for it.
  3. It looks like you created a new branch for this merge as we discussed, but you still have 150 changes in your history. Presumably you created the new branch based off the 'master' branch in your fork? I guess you need to make your fork master branch match origin master branch, or create the new branch off of origin master instead of off of master in your fork? I'm afraid I don't have much experience with forked repositories so I'm not sure what the correct procedure is. If you sort it out please let me know :-). EDIT: This doesn't have to be fixed for this change, just an FYI.

Copy link
Contributor

@mdbtucker mdbtucker left a comment

Choose a reason for hiding this comment

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

Sorry, getting used to the UI here. See my previous comment.

@JoeOster
Copy link
Contributor Author

JoeOster commented Apr 5, 2021

Sorry, getting used to the UI here. See my previous comment.
#1 - failed like I was suspecting, I will be getting it resolved
#2 will be resolved when the other PR gets merged, in the next day or two.
#3, yea I need to get that cleaned up

mdbtucker
mdbtucker previously approved these changes Apr 5, 2021
Copy link
Contributor

@mdbtucker mdbtucker left a comment

Choose a reason for hiding this comment

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

Approving on the assumption that this latest change fixes the CI problem.

@JoeOster
Copy link
Contributor Author

JoeOster commented Apr 8, 2021

stopped this path

@JoeOster JoeOster deleted the Add_sample_json branch April 19, 2021 16:18
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.

6 participants