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

update copy for code samples that require a bsp #11

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

whitepau
Copy link
Owner

Existing Sample Changes

Description

Readme change

Fixes Issue#

Type of change

Please delete options that are not relevant. Add a 'X' to the one that is applicable.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Implement fixes for ONSAM Jiras

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Command Line
  • oneapi-cli
  • Visual Studio
  • Eclipse IDE
  • VSCode
  • When compiling the compliler flag "-Wall -Wformat-security -Werror=format-security" was used

Copy link
Collaborator

@yuguen-intel yuguen-intel left a comment

Choose a reason for hiding this comment

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

Except the small comment, this looks good to me - I'll make a bulk edit out of it

provide cmake with the full path to your sample directory.
>**Note**: If you encounter any issues with long paths when compiling under Windows*, you may have to create your 'build' directory in a shorter path, for example `C:\samples\build`. You can then run cmake from that directory, and provide cmake with the full path to your sample directory, for example
> ```
> C:\samples\build> cmake -G "NMake Makefiles" C:\long\path\to\code\sample\CMakeLists.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

I find this part overkill, don't you think?

Copy link
Owner Author

Choose a reason for hiding this comment

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

i always find an example helpful.

If I found it overkill i wouldn't have written it :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah but here we are showing how to use cmake which is orthogonal to what the sample is trying to show.
We could also explain how to install cmake, etc.
I feel like we should expect people to know how to use the tools (e.g. we expect people to know how to open a terminal, navigate to the appropriate folder, etc.)
We give then the necessary info to run cmake in our particular context (the flags we use), but I don't view it as necessary to give them more general information such as how to run cmake from a different folder than the build directory that we just explained them above

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.

2 participants