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

Massive breaking changes in preparation for 2.0 #546

Closed

Conversation

achew22
Copy link
Collaborator

@achew22 achew22 commented Feb 10, 2018

This PR turned out to be an absolutely gigantic... thing. Since I'm making breaking changes why not break all the things.

Things I broke:

  • Emitting defaults
  • Now uses the jsonpb marshaller by default
  • Upgrade to swagger 2.4

Obviously this can't go in until swagger 2.4 is released, but we have a backlog of significant breaking changes that would improve the quality of the service dramatically. My plan is to cut one last 1.X branch release then merge merge merge and cut a 2.0

TODO

  • I need to add an upgrading section to the docs

@achew22 achew22 force-pushed the upgrade_swagger_codegen branch 2 times, most recently from 5aa0d42 to b1d75c2 Compare February 10, 2018 20:35
@thurt
Copy link
Contributor

thurt commented Feb 12, 2018

One of the recurring themes of this project has been trouble around the
default marshaller. This change modifies it to be more what people
expect when they first start the project.

1.  It emits the proto3 json style version of field names instead of the
    field name as it appeared in the .proto file.
2.  It emits zero values for fields. This means that if you have a field
    that is unset it will now have a value unlike before.

Upgrade to swagger-codegen 2.4.0

Also fix a regex-o in .travis.yml. + needed to be escaped.

Fixes: grpc-ecosystem#540, grpc-ecosystem#375, grpc-ecosystem#254, grpc-ecosystem#233
@achew22 achew22 changed the title Upgrade to swagger-codegen 2.3.1 Massive breaking changes in preparation for 2.0 Apr 9, 2018
@achew22 achew22 mentioned this pull request Apr 9, 2018
@johanbrandhorst johanbrandhorst added breaking change Will require a major version increment v2 labels Sep 9, 2018
@johanbrandhorst
Copy link
Collaborator

This seems to have suffered some bitrot, which is unfortunate for a large change like this as it'll be a pain to keep rebasing it.

@johanbrandhorst
Copy link
Collaborator

Other things I'd like for 2.0:

Output errors in the same format as the gRPC-Go status.Status, that is, simply use status.Convert followed by marshaller.Marshal.

@johanbrandhorst
Copy link
Collaborator

Another one: Adopt the correct behaviour for custom verbs (see #224 (comment)).

@johanbrandhorst
Copy link
Collaborator

Please rebase on master for new CI functionality. 😬

@stale
Copy link

stale bot commented Sep 9, 2019

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 Sep 9, 2019
@stale stale bot closed this Sep 16, 2019
@johanbrandhorst
Copy link
Collaborator

We'd love to have help getting this in for v2, though I'm not sure the original author will have the time.

@johanbrandhorst johanbrandhorst mentioned this pull request Apr 20, 2020
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Will require a major version increment cla: yes cleanup wontfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants