Skip to content

feat: support internal field #193

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

Merged
merged 1 commit into from
Apr 28, 2025
Merged

feat: support internal field #193

merged 1 commit into from
Apr 28, 2025

Conversation

sashamelentyev
Copy link
Collaborator

No description provided.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces support for internal fields by excluding them from the generated JSON schema. Key changes include:

  • Adding internal field detection functions in internal/gen/visibility.go.
  • Updating the schema generation logic in internal/gen/schema.go to skip internal fields.
  • Updating the proto examples and adding the visibility extension definitions.

Reviewed Changes

Copilot reviewed 5 out of 9 changed files in this pull request and generated no comments.

File Description
internal/gen/visibility.go New helper functions to detect internal fields using field visibility.
internal/gen/schema.go Schema generation now skips fields marked with internal visibility.
example/message.proto Added an internal field with the internal visibility annotation.
example/google/api/visibility.proto Added visibility extensions and definitions for proto field visibility.
Files not reviewed (4)
  • go.mod: Language not supported
  • internal/gen/_golden/internal_field.json: Language not supported
  • internal/gen/_golden/internal_field.yaml: Language not supported
  • internal/gen/_testdata/internal_field.textproto: Language not supported

@sashamelentyev sashamelentyev requested a review from Copilot April 28, 2025 17:47
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces support for internal fields by adding helper functions to determine field visibility and updating the schema generation to skip internal fields.

  • Added functions (isInternalField, isPreviewField, isFieldVisibilityIndicator) in internal/gen/visibility.go
  • Modified schema generation in internal/gen/schema.go to exclude internal (and non-preview) fields
  • Updated proto files to include an example usage of the new visibility extension

Reviewed Changes

Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.

File Description
internal/gen/visibility.go New helper functions to evaluate field visibility using the visibility protobuf extension
internal/gen/schema.go Modified schema generation to skip fields marked as internal (but not preview)
example/message.proto Added an internal field example with the new visibility extension
example/google/api/visibility.proto Added proto definitions for specifying field visibility restrictions
Files not reviewed (4)
  • go.mod: Language not supported
  • internal/gen/_golden/internal_field.json: Language not supported
  • internal/gen/_golden/internal_field.yaml: Language not supported
  • internal/gen/_testdata/internal_field.textproto: Language not supported
Comments suppressed due to low confidence (1)

internal/gen/schema.go:97

  • [nitpick] Consider caching the result of f.Desc.Options() in a variable to avoid calling it twice and to improve code readability.
isInternal := isInternalField(f.Desc.Options()) && !isPreviewField(f.Desc.Options())

Signed-off-by: Sasha Melentyev <[email protected]>
@ernado ernado merged commit d5cc052 into main Apr 28, 2025
10 checks passed
@tdakkota tdakkota deleted the internal-field branch April 28, 2025 21:16
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.

3 participants