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

names docstring: qualify non-exported cross-refs #52745

Merged
merged 2 commits into from
Feb 10, 2024
Merged

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Jan 4, 2024

It seems like cross-references to non-exported symbols, like ispublic, should be qualified — otherwise it is confusing since looking them up fails.

@stevengj stevengj added the domain:docs This change adds or pertains to documentation label Jan 4, 2024
@stevengj
Copy link
Member Author

stevengj commented Jan 4, 2024

(It would be nice if the docsystem did this automatically somehow.)

@@ -84,7 +84,7 @@ are also included. Names are returned in sorted order.
As a special case, all names defined in `Main` are considered \"public\",
since it is not idiomatic to explicitly mark names from `Main` as public.

See also: [`isexported`](@ref), [`ispublic`](@ref), [`@locals`](@ref Base.@locals), [`@__MODULE__`](@ref).
See also: [`Base.isexported`](@ref), [`Base.ispublic`](@ref), [`Base.@locals`](@ref Base.@locals), [`@__MODULE__`](@ref).
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe like this (I am not sure what the @ref Base.@locals does, so I might be guessing wrong)

Suggested change
See also: [`Base.isexported`](@ref), [`Base.ispublic`](@ref), [`Base.@locals`](@ref Base.@locals), [`@__MODULE__`](@ref).
See also: [`Base.isexported`](@ref), [`Base.ispublic`](@ref), [`Base.@locals`](@ref), [`@__MODULE__`](@ref).

@fingolfin
Copy link
Contributor

I resolved the conflict and applied my suggested change while doing so, but someone should who knows about @ref should double check.

@fingolfin fingolfin added the status:merge me PR is reviewed. Merge when all tests are passing label Feb 10, 2024
@vtjnash vtjnash merged commit 65ffa79 into master Feb 10, 2024
8 checks passed
@vtjnash vtjnash deleted the stevengj-patch-6 branch February 10, 2024 18:11
@inkydragon inkydragon removed the status:merge me PR is reviewed. Merge when all tests are passing label Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants