Skip to content

Add docstring to ConcurrencyViolationError #53733

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

Merged

Conversation

Seelengrab
Copy link
Contributor

This should cover the current usecases of this error.

@Seelengrab Seelengrab added docs This change adds or pertains to documentation multithreading Base.Threads and related functionality labels Mar 14, 2024
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

Should we add this to the docs/src files also?

@Seelengrab
Copy link
Contributor Author

Seelengrab commented Mar 14, 2024

What is recorded in those files?

@vtjnash
Copy link
Member

vtjnash commented Mar 14, 2024

The source for the https://docs.julialang.org/en/v1/ pages

@Seelengrab
Copy link
Contributor Author

Seelengrab commented Mar 14, 2024

I think it'd be good to have a dedicated "You're doing something weird with concurrency; if you see this error you most likely have a bug in your code" error, so yeah, having that listed there sounds good. I'll add it. Is there a specific section it should be listed in?

@gbaraldi
Copy link
Member

Multithreading maybe?

@Seelengrab
Copy link
Contributor Author

I'm not sure it's worth adding a whole big paragraph of prose though :/ If this were a personal project, I'd just have it in a reference list (kinda like here, except it'd only contain multi-threading related docstrings). It's not like there are big examples necessary for how to use it. We don't really have those kinds of section-specific lists in the manual, unfortunately.

@Seelengrab
Copy link
Contributor Author

Alright, I've added a short blurb to the section on data races, which seemed like the most appropriate place ConcurrencyViolationError should be mentioned.

@Seelengrab
Copy link
Contributor Author

Failures are due to not having a canonical place for the docstring-@ref of ConcurrencyViolationError. Do we have a place for multithreading docstrings to be shown that I can add this too?

@vtjnash
Copy link
Member

vtjnash commented Apr 16, 2024

Those seem to be split between docs/src/base/multi-threading.md and docs/src/base/parallel.md, with probably a preference towards the former for this, though either seems acceptable.

@Seelengrab
Copy link
Contributor Author

Ok, I've added it to parallel.md, since the error is not exclusive to multithreading but for concurrency in general. Let's see if CI likes it now.

@Seelengrab
Copy link
Contributor Author

Failures seem unrelated, so should be good to merge if the addition to the docs is ok.

@ViralBShah
Copy link
Member

Should we merge?

@Seelengrab
Copy link
Contributor Author

Seelengrab commented Jun 9, 2024

Like I said in May, if the failure is unrelated (and it seems like it is) and folks are fine with the wording, yes. It's just a doc change.

@ViralBShah ViralBShah merged commit ce38f08 into JuliaLang:master Jun 10, 2024
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 multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants