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

feat: add option to add description to tags #2939

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

same-id
Copy link
Contributor

@same-id same-id commented Oct 13, 2022

References to other Issues or PRs

Have you read the Contributing Guidelines?

Yes

Brief description of what is fixed or changed

Adding option to add description to tags by allowing specifying openapiv2 tags

Other comments

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.

Hi, thanks for your PR! This looks pretty neat to me, almost deceptively so. I'm going to rope in @ivucica for his thoughts on the changes since he's the original author.

@johanbrandhorst
Copy link
Collaborator

Looks like we ran into some CI errors

@same-id
Copy link
Contributor Author

same-id commented Oct 13, 2022

Hmm, seems like lint failed on removal of reserved fields, that is the correct process for that? Isn't it supposed to fail?

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 would ignore that lint error, we can merge over it. Your change looks right to me. The error here assumes that the field number was reserved because it has been removed (https://docs.buf.build/breaking/rules#reserved_message_no_delete), which is not the case here.

@same-id
Copy link
Contributor Author

same-id commented Oct 13, 2022

Cool, committed the suggestion as well

@kkolur
Copy link
Contributor

kkolur commented Oct 13, 2022

Thanks @same-id for this great work!

@johanbrandhorst can we add extensions to the Tag object as well, maybe in a separate PR?

@johanbrandhorst
Copy link
Collaborator

@johanbrandhorst can we add extensions to the Tag object as well, maybe in a separate PR?

I think this'd have to start as a separate issue since it's pretty broad scope, could you create one?

@kkolur
Copy link
Contributor

kkolur commented Oct 13, 2022

@johanbrandhorst can we add extensions to the Tag object as well, maybe in a separate PR?

I think this'd have to start as a separate issue since it's pretty broad scope, could you create one?

Created issue here

@kkolur
Copy link
Contributor

kkolur commented Oct 13, 2022

@same-id @johanbrandhorst what would happen in the case a user defines their own tags at root level of open api object but dont disable the default rendering of service tags? Would the user defined tags override the default service tag generation, or does it get appended to it? How should this work?

@johanbrandhorst
Copy link
Collaborator

@same-id @johanbrandhorst what would happen in the case a user defines their own tags at root level of open api object but dont disable the default rendering of service tags? Would the user defined tags override the default service tag generation, or does it get appended to it? How should this work?

The example added here already shows what happens - the tags are simply appended to the end of the service tags.

@johanbrandhorst
Copy link
Collaborator

Please rebase/merge on master and regenerate.

@same-id
Copy link
Contributor Author

same-id commented Oct 13, 2022

After I regenerate I'm having lots of unrelated swagger-codegen diffs, not sure why:

-                  "$ref": "#/definitions/examplepbNumericEnum"
+                  "$ref": "#/definitions/examplepbNumericEnum",
+                  "description": "Numeric enum description.",
+                  "title": "Numeric enum title"
                 },
                 "repeatedStringAnnotation": {
                   "type": "array",
@@ -2358,7 +2360,9 @@
                   "title": "Repeated nested object title"
                 },
                 "nestedAnnotation": {
-                  "$ref": "#/definitions/ABitOfEverythingNested"
+                  "$ref": "#/definitions/ABitOfEverythingNested",
+                  "description": "Nested object description.",
+                  "title": "Nested object title"
                 },

Can you try and execute swagger-codegen (version 2.4.8) on master and check if it's not in sync?

@johanbrandhorst
Copy link
Collaborator

You need to reinstall the plugin again, there was a change in the latest PR. Run make install before running the regeneration command.

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, and I chatted with @ivucica about this change, he's happy too.

@same-id
Copy link
Contributor Author

same-id commented Oct 14, 2022

Rebased

@kkolur
Copy link
Contributor

kkolur commented Oct 14, 2022

@same-id can we get this over the hump? I have an issue that depends on changes here and would love to get started on that. Great work btw!

@johanbrandhorst
Copy link
Collaborator

This failure is an expected false positive, I will merge as-is.

@johanbrandhorst johanbrandhorst merged commit 813bbcd into grpc-ecosystem:master Oct 14, 2022
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution @same-id!

@same-id
Copy link
Contributor Author

same-id commented Oct 18, 2022

Is there a community for grpc-gateway?
I would love to ask some general questions (mainly regarding integrations with redoc) and not sure where it's best to have that discussion.

@adambabik
Copy link
Collaborator

@same-id join Gophers Slack and find the grpc-gateway channel. I think that's the most active community currently.

another-rex referenced this pull request in google/osv.dev Nov 4, 2022
)

[![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://github.com/grpc-ecosystem/grpc-gateway)
| require | minor | `v2.11.3` -> `v2.13.0` |

---

### Release Notes

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

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

[Compare
Source](https://github.com/grpc-ecosystem/grpc-gateway/compare/v2.12.0...v2.13.0)

#### What's Changed

- Updated gRPC code Cancelled replaced with HTTP code 499 by
[@&#8203;tech-sumit](https://github.com/tech-sumit) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2957](https://github.com/grpc-ecosystem/grpc-gateway/pull/2957)
- fix: remove default service tag generation for methods by
[@&#8203;kkolur](https://github.com/kkolur) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2960](https://github.com/grpc-ecosystem/grpc-gateway/pull/2960)
- Use tag instead of has pin for SLSA generator by
[@&#8203;laurentsimon](https://github.com/laurentsimon) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2969](https://github.com/grpc-ecosystem/grpc-gateway/pull/2969)
- Add Conduit to adopters by
[@&#8203;hariso](https://github.com/hariso) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2981](https://github.com/grpc-ecosystem/grpc-gateway/pull/2981)
- feat(gen-openapiv2): support trailing comments by
[@&#8203;ionling](https://github.com/ionling) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2965](https://github.com/grpc-ecosystem/grpc-gateway/pull/2965)
- feat(gen-openapiv2): keep fields next to "$ref" fields by
[@&#8203;gostajonasson](https://github.com/gostajonasson) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2986](https://github.com/grpc-ecosystem/grpc-gateway/pull/2986)

#### New Contributors

- [@&#8203;tech-sumit](https://github.com/tech-sumit) made their first
contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2957](https://github.com/grpc-ecosystem/grpc-gateway/pull/2957)
- [@&#8203;hariso](https://github.com/hariso) made their first
contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2981](https://github.com/grpc-ecosystem/grpc-gateway/pull/2981)
- [@&#8203;ionling](https://github.com/ionling) made their first
contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2965](https://github.com/grpc-ecosystem/grpc-gateway/pull/2965)

**Full Changelog**:
grpc-ecosystem/grpc-gateway@v2.12.0...v2.13.0

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

[Compare
Source](https://github.com/grpc-ecosystem/grpc-gateway/compare/v2.11.3...v2.12.0)

#### What's Changed

- fix: support for oneof fields in request bodies by
[@&#8203;isbang](https://github.com/isbang) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2867](https://github.com/grpc-ecosystem/grpc-gateway/pull/2867)
- mux: calculate verb correctly for cases like DELETE /foo/bar:archive
when user provided wrong method by
[@&#8203;jonathaningram](https://github.com/jonathaningram) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2870](https://github.com/grpc-ecosystem/grpc-gateway/pull/2870)
- Update googleapis dependency by
[@&#8203;johanbrandhorst](https://github.com/johanbrandhorst) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2875](https://github.com/grpc-ecosystem/grpc-gateway/pull/2875)
- protoc-gen-openapiv2: RPC visibility setting transitively applied to
messages by [@&#8203;erademacher](https://github.com/erademacher) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2880](https://github.com/grpc-ecosystem/grpc-gateway/pull/2880)
- protoc-gen-openapiv2: Use json_name when generating required field
names by [@&#8203;patrick246](https://github.com/patrick246) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2885](https://github.com/grpc-ecosystem/grpc-gateway/pull/2885)
- fix: support service tags in OpenAPI config file
([#&#8203;2817](https://github.com/grpc-ecosystem/grpc-gateway/issues/2817))
by [@&#8203;y-takuya](https://github.com/y-takuya) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2858](https://github.com/grpc-ecosystem/grpc-gateway/pull/2858)
- feat: add option to disable rendering of service tags by
[@&#8203;kkolur](https://github.com/kkolur) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2928](https://github.com/grpc-ecosystem/grpc-gateway/pull/2928)
- fix: required properties of message type are required in OpenAPI by
[@&#8203;gostajonasson](https://github.com/gostajonasson) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2904](https://github.com/grpc-ecosystem/grpc-gateway/pull/2904)
- feat: add option to add description to tags by
[@&#8203;same-id](https://github.com/same-id) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2939](https://github.com/grpc-ecosystem/grpc-gateway/pull/2939)
- add extensions for Tag object by
[@&#8203;kkolur](https://github.com/kkolur) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2950](https://github.com/grpc-ecosystem/grpc-gateway/pull/2950)
- Make registry load packages deterministically by
[@&#8203;gonzaloserrano](https://github.com/gonzaloserrano) in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2945](https://github.com/grpc-ecosystem/grpc-gateway/pull/2945)

#### New Contributors

- [@&#8203;erademacher](https://github.com/erademacher) made their
first contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2880](https://github.com/grpc-ecosystem/grpc-gateway/pull/2880)
- [@&#8203;patrick246](https://github.com/patrick246) made their first
contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2885](https://github.com/grpc-ecosystem/grpc-gateway/pull/2885)
- [@&#8203;y-takuya](https://github.com/y-takuya) made their first
contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2858](https://github.com/grpc-ecosystem/grpc-gateway/pull/2858)
- [@&#8203;kkolur](https://github.com/kkolur) made their first
contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2928](https://github.com/grpc-ecosystem/grpc-gateway/pull/2928)
- [@&#8203;gostajonasson](https://github.com/gostajonasson) made their
first contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2904](https://github.com/grpc-ecosystem/grpc-gateway/pull/2904)
- [@&#8203;same-id](https://github.com/same-id) made their first
contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2939](https://github.com/grpc-ecosystem/grpc-gateway/pull/2939)
- [@&#8203;gonzaloserrano](https://github.com/gonzaloserrano) made
their first contribution in
[https://github.com/grpc-ecosystem/grpc-gateway/pull/2945](https://github.com/grpc-ecosystem/grpc-gateway/pull/2945)

**Full Changelog**:
grpc-ecosystem/grpc-gateway@v2.11.3...v2.12.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6am on monday" 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.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- 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://app.renovatebot.com/dashboard#github/google/osv.dev).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yNDEuOSIsInVwZGF0ZWRJblZlciI6IjM0LjEyLjAifQ==-->

Co-authored-by: Rex P <106129829+another-rex@users.noreply.github.com>
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