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

Update logging.jl to add docstrings for exported symbols #53299

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Update logging.jl to add docstrings for exported symbols #53299

merged 1 commit into from
Feb 13, 2024

Conversation

AllHailTheSheep
Copy link
Contributor

This is a part of issue #52725 and supersedes this pull request as I specified the incorrect base repo.

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.

Thank you!

These symbols are internal, but documenting internals is good. This PR changes

before:

help?> Base.CoreLogging.AboveMaxLevel
  │ Warning
  │
  │  The following bindings may be
  │  internal; they may change or be
  │  removed in future versions:
  │
  │    •  Base.CoreLogging
  │
  │    •  Base.CoreLogging.AboveMaxLevel

  No documentation found for private symbol.

  Base.CoreLogging.AboveMaxLevel is of type
  Base.CoreLogging.LogLevel.

  Summary
  ≡≡≡≡≡≡≡

  struct Base.CoreLogging.LogLevel

  Fields
  ≡≡≡≡≡≡

  level :: Int32

to

after:

help?> Base.CoreLogging.AboveMaxLevel
  │ Warning
  │
  │  The following bindings may be
  │  internal; they may change or be
  │  removed in future versions:
  │
  │    •  Base.CoreLogging
  │
  │    •  Base.CoreLogging.AboveMaxLevel

  AboveMaxLevel

  Alias for LogLevel(1_000_001).

Which is an unambiguous improvement. (With the public keyword it is clear that adding these docstrings does not make these symbols public)

@LilithHafner LilithHafner added docs This change adds or pertains to documentation merge me PR is reviewed. Merge when all tests are passing labels Feb 12, 2024
@vtjnash vtjnash merged commit 69f16a2 into JuliaLang:master Feb 13, 2024
6 of 9 checks passed
@AllHailTheSheep AllHailTheSheep deleted the AllHailTheSheep-julia-logging-docstrings branch February 13, 2024 02:41
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Feb 13, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants