Skip to content

Drop application/yaml content type#933

Merged
bobcallaway merged 3 commits intosigstore:mainfrom
Hayden-IO:drop-yaml
Jul 27, 2022
Merged

Drop application/yaml content type#933
bobcallaway merged 3 commits intosigstore:mainfrom
Hayden-IO:drop-yaml

Conversation

@Hayden-IO
Copy link
Copy Markdown
Contributor

@Hayden-IO Hayden-IO commented Jul 21, 2022 •
edited
Loading

This is a breaking change for any clients that are currently requesting
application/yaml.

This is the simplest fix to #593, as openapi randomly switches between
json and yaml responses when no content type is provided.

Fixes #593

Signed-off-by: Hayden Blauzvern hblauzvern@google.com

Summary

Release Note

Dropped support for YAML content type

Documentation

@Hayden-IO
Drop application/yaml content type
b417456 
This is a breaking change for any clients that are currently requesting
application/yaml.

This is the simplest fix to sigstore#593, as openapi randomly switches between
json and yaml responses when no content type is provided.

Fixes sigstore#593

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@Hayden-IO Hayden-IO requested a review from a team as a code owner July 21, 2022 18:21
@Hayden-IO Hayden-IO requested review from bobcallaway and dlorenc July 21, 2022 18:21
@Hayden-IO
Copy link
Copy Markdown
Contributor Author

Hayden-IO commented Jul 21, 2022

Looks like there is a difference between how the comments are generated. This should be ok.

@danishhansari
Copy link
Copy Markdown

danishhansari commented Jul 24, 2022

hey 👋 when i open a PR there was a dependabot that making unnecessary commit automatically i have mo idea about that please help me i am stuck

@bobcallaway
Copy link
Copy Markdown
Member

bobcallaway commented Jul 24, 2022

Looks like there is a difference between how the comments are generated. This should be ok.

I think there's one additional thing in the Makefile that needs addressed: --default-consumes application/json\;q=1 should be removed

@bobcallaway
Copy link
Copy Markdown
Member

bobcallaway commented Jul 24, 2022

hey wave when i open a PR there was a dependabot that making unnecessary commit automatically i have mo idea about that please help me i am stuck

@Ansari-Danish please open a separate issue instead of discussing on an unrelated PR. Thanks!

@Hayden-IO
remove default-consumes from Makefile
52220e9 
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@Hayden-IO
Copy link
Copy Markdown
Contributor Author

Hayden-IO commented Jul 24, 2022

@bobcallaway Updated the Makefile. Any advice on how to handle the client differences due to the comment? Can we just ignore the required check when merging?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 24, 2022 •
edited
Loading

Codecov Report

Merging #933 (5b7f32d) into main (cb5572d) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #933      +/-   ##
==========================================
+ Coverage   48.20%   48.35%   +0.15%     
==========================================
  Files          62       61       -1     
  Lines        5398     5383      -15     
==========================================
+ Hits         2602     2603       +1     
+ Misses       2513     2500      -13     
+ Partials      283      280       -3
Impacted Files Coverage Δ
pkg/sharding/ranges.go 47.27% <ø> (ø)
pkg/client/rekor_client.go 86.95% <100.00%> (ø)
pkg/types/alpine/v0.0.1/entry.go 56.72% <0.00%> (+1.26%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@Hayden-IO
Copy link
Copy Markdown
Contributor Author

Hayden-IO commented Jul 26, 2022

@bobcallaway Ran make clean; make, and got the same result.

@Hayden-IO
Fix comment generation
5b7f32d 
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@Hayden-IO
Copy link
Copy Markdown
Contributor Author

Hayden-IO commented Jul 27, 2022

Figured it out, I was using go 1.19 and not .18.

@bobcallaway bobcallaway merged commit 4da9b37 into sigstore:main Jul 27, 2022
@github-actions github-actions bot added this to the v1.0.0 milestone Jul 27, 2022
@Hayden-IO Hayden-IO deleted the drop-yaml branch July 27, 2022 18:36
bobcallaway pushed a commit to bobcallaway/rekor that referenced this pull request Aug 3, 2022
@Hayden-IO @bobcallaway
Drop application/yaml content type (sigstore#933)
d3fa7da 
* Drop application/yaml content type

This is a breaking change for any clients that are currently requesting
application/yaml.

This is the simplest fix to sigstore#593, as openapi randomly switches between
json and yaml responses when no content type is provided.

Fixes sigstore#593

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

* remove default-consumes from Makefile

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

* Fix comment generation

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bobcallaway bobcallaway bobcallaway approved these changes

@dlorenc dlorenc Awaiting requested review from dlorenc

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.0.0

Development

Successfully merging this pull request may close these issues.

Rekor API sometimes responds with YAML instead of JSON

4 participants

@Hayden-IO @danishhansari @bobcallaway @codecov-commenter