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

Generate docs of enums and rpc clients, some refactoring #909

Merged
merged 10 commits into from
Jan 8, 2024

Conversation

osa1
Copy link
Member

@osa1 osa1 commented Dec 29, 2023

This implements generating doc comments for enum types, enum values, rpc types,
and rpc methods.

Existing code for generating doc comments is refactored: the comment generator
handles trailing whitespace so we don't have to handle it in the call sites.
Comment generator is documented with the invariants: it never returns an empty
string (returns null instead) and never returns lines with trailing
whitespace.

To be able to test comment generating, a new kind of golden file test is added.
The difference from the other golden file tests is that this file is compiled
from .proto source instead of from a hand-crafted descriptor. This is because
attaching comments to proto field and types by hand is difficult and error
prone.

Fixes the comment part of #900.


cl/596840489

Currently we have 3 places where we generate a doc comment, and each of
these does it differently.

With this they now generate the comments the same way. The doc comment
generator functions are documented with the invariants, namely that they
never return an empty string and the output is always trimmed (i.e. no
trailing whitespace).
Copy link

github-actions bot commented Dec 29, 2023

Package publishing

Package Version Status Publish tag (post-merge)
package:protobuf 4.0.0-dev ready to publish protobuf-v4.0.0-dev
package:protoc_plugin 22.0.0-dev ready to publish protoc_plugin-v22.0.0-dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@osa1 osa1 changed the title Refactor and test doc comment generation Refactor and test doc comment generation, generate docs for enum variants and services Dec 29, 2023
@osa1 osa1 changed the title Refactor and test doc comment generation, generate docs for enum variants and services Generate docs of enums and rpc clients, some refactoring Dec 29, 2023
@osa1
Copy link
Member Author

osa1 commented Jan 3, 2024

@sigurdm for field comments, any thoughts on which accessors should have the comments copied?

Currently for a field x we generate these public members:

  • get x
  • set x
  • hasX
  • clearX

The documentation is currently copied only to get x, but deprecation annotations are added to all of these (in master branch, not with #908).

If duplicating potentially large comment blocks 4 times is not a problem I think we should add docs to all.

@sigurdm
Copy link
Collaborator

sigurdm commented Jan 4, 2024

@sigurdm for field comments, any thoughts on which accessors should have the comments copied?

Really good question. I don't think the setter should have it: https://dart.dev/effective-dart/documentation#dont-write-documentation-for-both-the-getter-and-setter-of-a-property

Perhaps the hasX should just have a referring comment along the lines of:

Checks for presence of [X].

True if [X] has been set.

And clearX:

Unsets [X].

@osa1
Copy link
Member Author

osa1 commented Jan 4, 2024

Makes sense, and thanks for the reference.

In that case I think we don't have to change anything in this PR.

I'm not sure if "clearX" and "hasX" docs are necessary, but if we want them we can add them separately.

@osa1 osa1 merged commit 9a408a7 into master Jan 8, 2024
16 of 18 checks passed
@osa1 osa1 deleted the osa1/refactor_doc_comments branch January 8, 2024 11:12
osa1 added a commit that referenced this pull request Jan 8, 2024
…d values, messages (#908)

Similar to #909, a new kind of golden test is added here where the test input is
not a hand-crafted message descriptor but an actual .proto file.
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jan 9, 2024
…, shelf, tools, webdev

Revisions updated by `dart tools/rev_sdk_deps.dart`.

async (https://github.com/dart-lang/async/compare/9924570..e83d054):
  e83d054  2024-01-08  Futuristic Goo  Fix typo (dart-lang/async#262)

ecosystem (https://github.com/dart-lang/ecosystem/compare/dc44e82..1e2785d):
  1e2785d  2024-01-09  Jacob MacDonald  fix saving of comment ids to disk (dart-lang/ecosystem#221)
  244a28d  2024-01-09  Moritz  Update publish.yaml (dart-lang/ecosystem#217)
  bab9833  2024-01-09  Moritz  Fix health commenting (dart-lang/ecosystem#219)
  f87e6f4  2024-01-08  Moritz  Update health workflow (dart-lang/ecosystem#216)
  a58c7d8  2024-01-03  Moritz  Fix `labeler.yml` (dart-lang/ecosystem#214)

leak_tracker (https://github.com/dart-lang/leak_tracker/compare/3d4c0d6..4a5b077):
  4a5b077  2024-01-09  Polina Cherkasova  Enhance scripting. (dart-lang/leak_tracker#204)
  e7094f4  2024-01-08  Polina Cherkasova  Ignore test helpers. (dart-lang/leak_tracker#196)
  6591934  2024-01-03  Polina Cherkasova  Handle deprecation in Flutter. (dart-lang/leak_tracker#203)

markdown (https://github.com/dart-lang/markdown/compare/d2e7903..7fdfa55):
  7fdfa55  2024-01-01  dependabot[bot]  Bump actions/stale from 8.0.0 to 9.0.0 (dart-lang/markdown#571)
  5fab3a7  2023-12-19  Alex Li  ✨ Introduce AlertBlockSyntax (dart-lang/markdown#570)

native (https://github.com/dart-lang/native/compare/0605d9a..14f6da1):
  14f6da1d  2024-01-09  Simon Binder  Support `@Native` fields and `addressOf` (dart-lang/native#860)

protobuf (https://github.com/dart-lang/protobuf/compare/20ec685..a293fb9):
  a293fb9  2024-01-08  Ömer Sinan Ağacan  Handle deprecated options in grpc services and methods, enum types and values, messages (google/protobuf.dart#908)
  9a408a7  2024-01-08  Ömer Sinan Ağacan  Generate docs of enums and rpc clients, some refactoring (google/protobuf.dart#909)
  c4fd596  2024-01-06  Ömer Sinan Ağacan  Export GeneratedMessageGenericExtensions in generated files (google/protobuf.dart#907)

shelf (https://github.com/dart-lang/shelf/compare/733588f..823966f):
  823966f  2024-01-03  Moritz  Fix `labeler.yml` (dart-lang/shelf#403)

tools (https://github.com/dart-lang/tools/compare/2f59ab4..8ffc077):
  8ffc077  2024-01-03  Moritz  Fix `labeler.yml` (dart-lang/tools#224)

webdev (https://github.com/dart-lang/webdev/compare/b2405cb..c08a65c):
  c08a65c9  2024-01-09  Elliott Brooks  Loosen `vm_service` constraints and prepare DWDS for release to 23.1.1 (dart-lang/webdev#2329)
  651bdae6  2024-01-08  Derek Xu  Make FrontendServerClient start the frontend server from AOT snapshot by default (dart-lang/webdev#2263)
  4d1de266  2024-01-03  Elliott Brooks  Prepare DWDS for release to version 23.1.0 (dart-lang/webdev#2328)

Change-Id: I4d7fd994cc54ac2d72335c3ebf40710f3bd020e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345366
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants