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

New ignore_comments option for excluding proto comments from OpenAPI output #3252

Merged

Conversation

keyz
Copy link
Contributor

@keyz keyz commented Mar 20, 2023

References to other Issues or PRs

Fixes #2684

Have you read the Contributing Guidelines?

Yes

Brief description of what is fixed or changed

It's pretty common to have proto comments that are irrelevant to OpenAPI output. For instance, if you have a // buf:lint:ignore comment, the only way to exclude that today is to manually add an override via grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field per #2684 (comment).

In this PR I'm adding a new config option: ignore_comments. It's disabled by default and only affects the OpenAPI output; when you enable the option, all proto comments are excluded from the output. With the new option on, annotations can still be explicitly added via grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field.

Let me know if you have any feedback!

@google-cla
Copy link

google-cla bot commented Mar 20, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@keyz keyz force-pushed the ignore-proto-comments-option branch from 505b7b0 to 6033884 Compare March 20, 2023 21:54
Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

I can actually get behind this option. It's consistent and reasonably isolated from other functions. I would request three things:

  1. Lets just call it ignore_comments.
  2. Please add a check to startup that errors if both ignore_comments and use_go_template is set. These are mutually exclusive options.
  3. Lets add something to the docs, specifically we usually have a section for each flag in https://github.com/grpc-ecosystem/grpc-gateway/blob/main/docs/docs/mapping/customizing_openapi_output.md.
    Thank you!

protoc-gen-openapiv2/main.go Outdated Show resolved Hide resolved
@keyz
Copy link
Contributor Author

keyz commented Mar 25, 2023

@johanbrandhorst sounds good -- just updated the PR. Thank you!

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@keyz keyz changed the title [RFC] New ignore_proto_comments option for excluding proto comments from OpenAPI output New ignore_comments option for excluding proto comments from OpenAPI output Mar 25, 2023
@johanbrandhorst johanbrandhorst merged commit 6f82d48 into grpc-ecosystem:main Mar 26, 2023
another-rex pushed a commit to google/osv.dev that referenced this pull request Jun 26, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[github.com/grpc-ecosystem/grpc-gateway/v2](https://togithub.com/grpc-ecosystem/grpc-gateway)
| require | minor | `v2.15.2` -> `v2.16.0` |
| [jekyll-feed](https://togithub.com/jekyll/jekyll-feed) | | minor |
`0.15.1` -> `0.17.0` |

---

### Release Notes

<details>
<summary>grpc-ecosystem/grpc-gateway</summary>

###
[`v2.16.0`](https://togithub.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.16.0)

[Compare
Source](https://togithub.com/grpc-ecosystem/grpc-gateway/compare/v2.15.2...v2.16.0)

#### What's Changed

- Upgrade some deps by
[@&#8203;johanbrandhorst](https://togithub.com/johanbrandhorst) in
[grpc-ecosystem/grpc-gateway#3214
- add note about incompatibility of remote plugins and external configu…
by [@&#8203;DGuhr](https://togithub.com/DGuhr) in
[grpc-ecosystem/grpc-gateway#3220
- Fix tests by
[@&#8203;johanbrandhorst](https://togithub.com/johanbrandhorst) in
[grpc-ecosystem/grpc-gateway#3234
- use_allof_for_refs: extend to example field by
[@&#8203;igor-tsiglyar](https://togithub.com/igor-tsiglyar) in
[grpc-ecosystem/grpc-gateway#3240
- New `ignore_comments` option for excluding proto comments from OpenAPI
output by [@&#8203;keyz](https://togithub.com/keyz) in
[grpc-ecosystem/grpc-gateway#3252
- Only allow POST -> GET path length fallback by
[@&#8203;nacx](https://togithub.com/nacx) in
[grpc-ecosystem/grpc-gateway#3272
- Remove GHA workflow that pushes to BSR by
[@&#8203;unmultimedio](https://togithub.com/unmultimedio) in
[grpc-ecosystem/grpc-gateway#3279
- fix: enum should be int when enums_as_int = true by
[@&#8203;hnlq715](https://togithub.com/hnlq715) in
[grpc-ecosystem/grpc-gateway#3167
- doc: Add OpenTelemetry go tracing example by
[@&#8203;iamrajiv](https://togithub.com/iamrajiv) in
[grpc-ecosystem/grpc-gateway#3309
- Switch from aliased field_mask package to canonical package by
[@&#8203;liggitt](https://togithub.com/liggitt) in
[grpc-ecosystem/grpc-gateway#3317
- Replace deprecated command with environment file by
[@&#8203;jongwooo](https://togithub.com/jongwooo) in
[grpc-ecosystem/grpc-gateway#3319
- changed block from code to paragraph by
[@&#8203;shashaneRanasinghe](https://togithub.com/shashaneRanasinghe) in
[grpc-ecosystem/grpc-gateway#3330
-
feat([#&#8203;3238](https://togithub.com/grpc-ecosystem/grpc-gateway/issues/3238)):Support
map in query parameters by
[@&#8203;li31727](https://togithub.com/li31727) in
[grpc-ecosystem/grpc-gateway#3323
- A few fixes for streaming responses by
[@&#8203;johanbrandhorst](https://togithub.com/johanbrandhorst) in
[grpc-ecosystem/grpc-gateway#3335

#### New Contributors

- [@&#8203;DGuhr](https://togithub.com/DGuhr) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3220
- [@&#8203;xieyuschen](https://togithub.com/xieyuschen) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3231
- [@&#8203;igor-tsiglyar](https://togithub.com/igor-tsiglyar) made their
first contribution in
[grpc-ecosystem/grpc-gateway#3240
- [@&#8203;KiK0S](https://togithub.com/KiK0S) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3247
- [@&#8203;keyz](https://togithub.com/keyz) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3252
- [@&#8203;nacx](https://togithub.com/nacx) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3272
- [@&#8203;unmultimedio](https://togithub.com/unmultimedio) made their
first contribution in
[grpc-ecosystem/grpc-gateway#3279
- [@&#8203;hnlq715](https://togithub.com/hnlq715) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3167
- [@&#8203;liggitt](https://togithub.com/liggitt) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3317
- [@&#8203;jongwooo](https://togithub.com/jongwooo) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3319
- [@&#8203;shashaneRanasinghe](https://togithub.com/shashaneRanasinghe)
made their first contribution in
[grpc-ecosystem/grpc-gateway#3330

**Full Changelog**:
grpc-ecosystem/grpc-gateway@v2.15.2...v2.15.3

</details>

<details>
<summary>jekyll/jekyll-feed</summary>

###
[`v0.17.0`](https://togithub.com/jekyll/jekyll-feed/blob/HEAD/History.markdown#&#8203;0170--2022-10-14)

[Compare
Source](https://togithub.com/jekyll/jekyll-feed/compare/v0.16.0...v0.17.0)

##### Documentation

- Update CI status badge
([#&#8203;363](https://togithub.com/jekyll/jekyll-feed/issues/363))

##### Development Fixes

- Add Ruby 3.1 to the CI matrix
([#&#8203;365](https://togithub.com/jekyll/jekyll-feed/issues/365))

##### Minor Enhancements

- Allow disabling of jekyll-feed while in development
([#&#8203;370](https://togithub.com/jekyll/jekyll-feed/issues/370))

###
[`v0.16.0`](https://togithub.com/jekyll/jekyll-feed/blob/HEAD/History.markdown#&#8203;0160--2022-01-03)

[Compare
Source](https://togithub.com/jekyll/jekyll-feed/compare/v0.15.1...v0.16.0)

##### Minor Enhancements

- Add support for `page.description` in front matter to become entry
`<summary>`
([#&#8203;297](https://togithub.com/jekyll/jekyll-feed/issues/297))

##### Bug Fixes

- Fold private methods into the `:render` method as local variables
([#&#8203;327](https://togithub.com/jekyll/jekyll-feed/issues/327))
- Check `post.categories` instead of `post.category`
([#&#8203;357](https://togithub.com/jekyll/jekyll-feed/issues/357))
- Switched xml_escape for `<![CDATA[]]>` for post content
([#&#8203;332](https://togithub.com/jekyll/jekyll-feed/issues/332))

##### Development Fixes

- Add Ruby 3.0 to CI
([#&#8203;337](https://togithub.com/jekyll/jekyll-feed/issues/337))
- Lock RuboCop to v1.18.x
([#&#8203;348](https://togithub.com/jekyll/jekyll-feed/issues/348))
- Add workflow to release gem via GH Action
([#&#8203;355](https://togithub.com/jekyll/jekyll-feed/issues/355))

##### Documentation

- Use `.atom` extension in documented examples since we write an Atom
feed ([#&#8203;359](https://togithub.com/jekyll/jekyll-feed/issues/359))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6am on wednesday" in timezone
Australia/Sydney, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/google/osv.dev).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTUuMiIsInVwZGF0ZWRJblZlciI6IjM1LjEzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIn0=-->
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.

Ignore buf lint comment in swagger definition title
2 participants