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.

Sign up for GitHub

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

Jump to bottom

Extend google.protobuf.EnumOptions #2855

Closed
wants to merge 7 commits into from
Closed

Extend google.protobuf.EnumOptions #2855

wants to merge 7 commits into from

Conversation

MakDon
Copy link
Contributor

@MakDon MakDon commented Aug 18, 2022

Close #2671

MakDon added 4 commits August 18, 2022 22:55
@MakDon
feat: Extend google.protobuf.EnumOptions
ad68902 
gen: annotations pb

docs and proto updates of enum options

regenerate files

avoid updateswaggerObjectFromJSONSchema overwrite openapiSchemaObject
with empty string

regenerate files
@MakDon
style: add newline at the end of file
e7e4517
@MakDon
fix: s.Default is overwritten with empty string
7d2dad2
@MakDon
regenerate files
6f1ea4f
@MakDon
Copy link
Contributor Author

MakDon commented Aug 18, 2022

It seems that external_docs is not rendered correctly. I am trying to figure why.

johanbrandhorst
johanbrandhorst requested changes Aug 18, 2022
View reviewed changes
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.

This looks good, with the exception of those required arrays! I'm going to rope in @ivucica who has done a lot of work in this domain for a review as well.

examples/internal/proto/examplepb/a_bit_of_everything.swagger.json
Comment on lines +6500 to +6504
"required": [
"uuid",
"int64Value",
"doubleValue"
],
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't look right, this should only apply to the messages that contain these fields. A bug?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It may be worthwhile for you to rebase on main, we just merged a PR that did some work around required fields.

@MakDon MakDon changed the title Issue 2671 Extend google.protobuf.EnumOptions Aug 19, 2022
@kostyay
Copy link

kostyay commented Oct 6, 2022

any progress on this? i'm also interested in this

@MakDon
Copy link
Contributor Author

MakDon commented Oct 8, 2022

any progress on this? i'm also interested in this

Still waiting for review from @ivucica

@johanbrandhorst
Copy link
Collaborator

Could you also please rebase/merge in master?

@stale
Copy link

stale bot commented Jun 18, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 18, 2023
@MakDon
Copy link
Contributor Author

MakDon commented Mar 8, 2024

Close PR for it is stale now.

@MakDon MakDon closed this Mar 8, 2024
@hown3d hown3d mentioned this pull request Nov 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johanbrandhorst johanbrandhorst johanbrandhorst requested changes

@ivucica ivucica Awaiting requested review from ivucica

Assignees
No one assigned
Labels
wontfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extend google.protobuf.EnumOptions
3 participants
@MakDon @kostyay @johanbrandhorst