julia icon indicating copy to clipboard operation
julia copied to clipboard

Add docstring to `ConcurrencyViolationError`

Open Seelengrab opened this issue 1 year ago • 10 comments

This should cover the current usecases of this error.

Seelengrab avatar Mar 14 '24 15:03 Seelengrab

What is recorded in those files?

Seelengrab avatar Mar 14 '24 16:03 Seelengrab

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

vtjnash avatar Mar 14 '24 16:03 vtjnash

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?

Seelengrab avatar Mar 14 '24 17:03 Seelengrab

Multithreading maybe?

gbaraldi avatar Mar 14 '24 21:03 gbaraldi

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 avatar Mar 14 '24 21:03 Seelengrab

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 avatar Apr 06 '24 07:04 Seelengrab

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?

Seelengrab avatar Apr 08 '24 06:04 Seelengrab

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.

vtjnash avatar Apr 16 '24 20:04 vtjnash

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 avatar Apr 21 '24 15:04 Seelengrab

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

Seelengrab avatar May 09 '24 12:05 Seelengrab

Should we merge?

ViralBShah avatar Jun 08 '24 17:06 ViralBShah

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.

Seelengrab avatar Jun 09 '24 07:06 Seelengrab