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

[docs] document sanitizers #4365

Merged
merged 4 commits into from
Jun 14, 2021
Merged

[docs] document sanitizers #4365

merged 4 commits into from
Jun 14, 2021

Conversation

@StrikerRUS StrikerRUS added the doc label Jun 9, 2021
@StrikerRUS StrikerRUS requested a review from jameslamb as a code owner June 9, 2021 22:40
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks for this useful addition to the documentation! Please see my comments about making this copy-pastable.

docs/Installation-Guide.rst Outdated Show resolved Hide resolved
@StrikerRUS
Copy link
Collaborator Author

@jameslamb Thanks for your review!

Please see my comments about making this copy-pastable.

I thought about it a lot and couldn't find a good variant. Unfortunately, your proposal is not fully copy-pastable as well due to that thread sanitizer is not compatible with others. Maybe you have any other variants?

@jameslamb
Copy link
Collaborator

Maybe you have any other variants?

I would prefer then to keep my suggestion but just not include thread in the list passed to -DENABLED_SANITIZERS

like

To enable them add ``-DUSE_SANITIZER=ON -DENABLED_SANITIZERS="address;leak;undefined"`` to CMake flags.

@StrikerRUS
Copy link
Collaborator Author

TBH, I don't like this variant very much. But because I'm out of ides for more general and clear wording, I'm going to accept your suggestion.

@StrikerRUS StrikerRUS requested a review from jameslamb June 14, 2021 12:31
@StrikerRUS StrikerRUS merged commit df79713 into master Jun 14, 2021
@StrikerRUS StrikerRUS deleted the sanitizer_docs branch June 14, 2021 19:15
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants