Skip to content

Conversation

LilithHafner
Copy link
Member

Implements the first step in #57012 (comment). See the discussion there for more info. Once this merges, #57012 will implement the recommended method throughout base and in-tree stdlibs.

@LilithHafner LilithHafner added the docs This change adds or pertains to documentation label Feb 28, 2025
@nsajko
Copy link
Member

nsajko commented Feb 28, 2025

Perhaps also add to the style guide in the manual?

@LilithHafner
Copy link
Member Author

Perhaps also add to the style guide in the manual?

Yep! That's #57580

@nickrobinson251
Copy link
Contributor

thumbs down because #57012 (comment) (sorry i didn't see this discussion til this PR, so understand this might have been decided, but i do think it could have an unintended consequence if it spreads an anti-pattern in code itself)

@digital-carver

This comment was marked as resolved.

@LilithHafner LilithHafner added the triage This should be discussed on a triage call label Mar 13, 2025
@LilithHafner
Copy link
Member Author

@nickrobinson251, thank you for sharing your concern that using :: on function signatures in docstrings might lead to folks inappropriately using :: on function definitions in code. I appreciate hearing that context and reason why this documentation approach is imperfect. I am going to go ahead and merge in the face of that risk because having a consistent pattern using :: for types and -> for values is better than an inconsistent mix which is what we have now and because no better (imo) consistent pattern has been proposed.

Some additional factors I'm considering here are

  • Triage (with quorum) approved this plan 3 weeks ago before we saw your concern
  • At triage last week (without quorum(IIRC it was just @JeffBezanson and I) we did not find the risk of broken mimicry to be compelling enough to use a different syntax in docs.
  • @StefanKarpinski has approved this
  • Return type annotations (conversions) on function signatures can be used correctly. This is not strictly an anti-pattern.
  • My intuition and subjective sense of style tell me that this is the right choice.
  • This sort of syntactic choice attracts a lot of diverse and subjective opinions giving maintainers the choice to proceed without full consensus or to stall in discussion for a long time/in perpetuity. I'm choosing the former.

@LilithHafner LilithHafner added merge me PR is reviewed. Merge when all tests are passing and removed triage This should be discussed on a triage call labels Mar 23, 2025
@IanButterworth IanButterworth merged commit 99a7647 into master Mar 23, 2025
9 checks passed
@IanButterworth IanButterworth deleted the lh/docstring-return branch March 23, 2025 15:50
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Mar 23, 2025
LilithHafner added a commit that referenced this pull request Mar 26, 2025
In cases where a function is documented as 
```
    function(arg::ArgT, arg2::Arg2T) -> RetT

...
```
I either switched ` -> ` to `::` or switched `RetT` to `ret_name::RetT`.

From the recommendation and justification from @nsajko here:
#56978 (comment) and
applied throughout the repo.

As documented here #57583

Also includes some minor changes to touched lines (e.g. removing annotations that are just clutter)
serenity4 pushed a commit to serenity4/julia that referenced this pull request May 1, 2025
In cases where a function is documented as 
```
    function(arg::ArgT, arg2::Arg2T) -> RetT

...
```
I either switched ` -> ` to `::` or switched `RetT` to `ret_name::RetT`.

From the recommendation and justification from @nsajko here:
JuliaLang#56978 (comment) and
applied throughout the repo.

As documented here JuliaLang#57583

Also includes some minor changes to touched lines (e.g. removing annotations that are just clutter)
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.

6 participants