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

clarify assert doc entry #45998

Merged
merged 8 commits into from
Oct 31, 2023
Merged

clarify assert doc entry #45998

merged 8 commits into from
Oct 31, 2023

Conversation

lmiq
Copy link
Contributor

@lmiq lmiq commented Jul 11, 2022

The @assert doc entry is somewhat confusing, first appearing to encourage then discourage its use. This attempts to clarify it for the general user.

The `@assert` doc entry is somewhat confusing, first appearing to encourage then discourage its use. This attempts to clarify it for the general user.
@ViralBShah ViralBShah added the docs This change adds or pertains to documentation label Jul 11, 2022
@kshyatt
Copy link
Contributor

kshyatt commented Jul 11, 2022

Is there a way to preserve the warning about side effects? This aspect seems important if the function opens a file or something like that.

reintroduced the side-effects comment
@lmiq
Copy link
Contributor Author

lmiq commented Jul 11, 2022

I tried to rephrase it a little to improve the proposal, and reintroduced the side-effects comment, as I didn't understand that it referred to something technical and specific. I am not sure if the general user will understand the "side-effects" part.

base/error.jl Outdated Show resolved Hide resolved
base/error.jl Outdated Show resolved Hide resolved
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

@assert 7 throws. Perhaps we should mention @assert expr will throw unless expr evaluates to true.

base/error.jl Outdated Show resolved Hide resolved
base/error.jl Outdated Show resolved Hide resolved
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 30, 2023
@IanButterworth IanButterworth merged commit 2b73a1d into JuliaLang:master Oct 31, 2023
2 checks passed
@IanButterworth IanButterworth removed the merge me PR is reviewed. Merge when all tests are passing label Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants