Skip to content

Changed environment-variable.md to add julia_num_threads description and edits to object.md to add a warning of outdated info#48975

Closed
TakshDhabalia wants to merge 3 commits intoJuliaLang:masterfrom
TakshDhabalia:master
Closed

Changed environment-variable.md to add julia_num_threads description and edits to object.md to add a warning of outdated info#48975
TakshDhabalia wants to merge 3 commits intoJuliaLang:masterfrom
TakshDhabalia:master

Conversation

@TakshDhabalia
Copy link
Copy Markdown
Contributor

Added some lines about Julia_Num_Threads in Environmental variables as per the open issue 48960 ,not added the other suggestion . open to suggestions .

@TakshDhabalia
Copy link
Copy Markdown
Contributor Author

also added a small change to the docs of object.md to put out of date till we update the documentation of the allocobj and other out of date stuff as per issue #48784 and the comments of the same

@TakshDhabalia TakshDhabalia changed the title Changed environment-variable.md to add julia_num_threads description Changed environment-variable.md to add julia_num_threads description and edits to object.md to add a warning of outdated info Mar 12, 2023
@Tokazama
Copy link
Copy Markdown
Contributor

This helps people not go down a rabbit whole trying to figure out this methods, but it really just makes it seem like this entire section is useless now.

As long as the relevant methods for these aren't changing drastically anytime soon we should probably just have the actual method names updated with a warning that they are internal and an unstable API.

@TakshDhabalia
Copy link
Copy Markdown
Contributor Author

Hey, thanks for the feedback @Tokazama . Could you suggest what I do for this? Just update the out of date mentions Or something else?

@Tokazama
Copy link
Copy Markdown
Contributor

It's a bit tricky figuring out what to do here because even the methods themselves aren't very well documented. You can sort of go off of what methods were important enough to include in "jl_exported_funcs.inc" but I'm still not sure. @MichaelHatherly appears to be the last person in the git blame on that line (7 years ago). He may have some feedback.

@MichaelHatherly
Copy link
Copy Markdown
Member

@MichaelHatherly appears to be the last person in the git blame on that line (7 years ago). He may have some feedback.

Those changes would have been from #18588 which was just part of the mass conversion from rst to md for the docs. So content-wise for this particular change I'd not have much to say on them.


!!! note "Out of date Warning"
The documentation and usage of `jl_gc_allocobj` may be out of date

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps it would be better to split into separate pr.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure, that would be better then merging in one pr to avoid confusion

@inkydragon
Copy link
Copy Markdown
Member

You can sort of go off of what methods were important enough to include in "jl_exported_funcs.inc" but I'm still not sure.

In fact, jl_gc_allocobj is a function that has been exported but rarely used, and jl_gc_alloc has not been exported but is heavily used.

So devdoc and some internal api are already a rabbit hole.

@inkydragon inkydragon added the docs This change adds or pertains to documentation label Mar 14, 2023
@Tokazama
Copy link
Copy Markdown
Contributor

I agree that the environment variables portion should be put in a separate PR. I think there will need to be several steps to resolve updating docs related to all C code.

I think the first step is updating contributor guidelines for C code. Even something as general as "See Base.ccall_wrapper_function or details" so that there's some relevant documentation for methods that are critical parts of any API.

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.

4 participants