Skip to content

Add extra documentation to Enum to point to EnumName. #53164

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

Closed
wants to merge 3 commits into from

Conversation

Hixie
Copy link
Contributor

@Hixie Hixie commented Aug 9, 2023

I keep forgetting you can use .name because the only sign that it exists in the Enum documentation is the in-passing mention that Enum has an extension, without mentioning why. This add an explicit callout to the Enum class definition.

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR, as far as I can tell.

I keep forgetting you can use `.name` because the only sign that it exists in the `Enum` documentation is the in-passing mention that `Enum` has an extension, without mentioning why. This add an explicit callout to the `Enum` class definition.
@copybara-service
Copy link

Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at:

https://dart-review.googlesource.com/c/sdk/+/319463

Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly.

Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR).

@@ -10,6 +10,9 @@ part of dart.core;
/// introduced using an `enum` declaration.
/// Non-platform classes cannot extend or mix in this class.
/// Concrete classes cannot implement the interface.
///
/// The string name of a value is available via the `name` property
Copy link
Member

@lrhn lrhn Aug 10, 2023

Choose a reason for hiding this comment

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

I'd go with

/// The intentifier used to name an `enum` value is available as a `String`, 
/// via the [`name`][EnumName.name] extension property on the `enum` value.

(If that's thow you link to other members without their full name in DartDoc. Where is the DartDoc syntax specification?)

Copy link
Member

@parlough parlough Aug 10, 2023

Choose a reason for hiding this comment

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

The grammar is currently documented at https://github.com/dart-lang/dartdoc/wiki/dartdoc-comment-references, but I don't think the doc is super well maintained.

We have a few issues open to properly document it on dart.dev but we haven't had a chance yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if that works, but I'm happy to change it to that if you would prefer. Let me know. :-)

@mraleph
Copy link
Member

mraleph commented Oct 19, 2023

@Hixie would you like to follow up on the comments or should we close the PR?

@Hixie
Copy link
Contributor Author

Hixie commented Oct 23, 2023

I was awaiting clarification about whether to apply @lrhn's suggestion or not. Is that the preferred syntax? It wasn't clear to me if that was what was being suggested or not.

@Hixie
Copy link
Contributor Author

Hixie commented Oct 23, 2023

I applied the change but without the part that's risky (which has the side-effect of being more explicit, which is probably good anyway).

@copybara-service
Copy link

https://dart-review.googlesource.com/c/sdk/+/319463 has been updated with the latest commits from this pull request.

1 similar comment
@copybara-service
Copy link

https://dart-review.googlesource.com/c/sdk/+/319463 has been updated with the latest commits from this pull request.

@mraleph
Copy link
Member

mraleph commented Nov 20, 2023

@lrhn friendly ping

Copy link

https://dart-review.googlesource.com/c/sdk/+/319463 has been updated with the latest commits from this pull request.

1 similar comment
Copy link

https://dart-review.googlesource.com/c/sdk/+/319463 has been updated with the latest commits from this pull request.

@Hixie Hixie closed this Jan 19, 2024
copybara-service bot pushed a commit that referenced this pull request Jan 23, 2024
Closes #53164

GitOrigin-RevId: 0c96101
Change-Id: I26391dddde87cdea0cc6ae4a70d04be1a48576f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319463
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: Lasse Nielsen <[email protected]>
Reviewed-by: Nate Bosch <[email protected]>
Commit-Queue: Lasse Nielsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants