feat: Inject OpenAPI deprecations safely#4286
Conversation
gmlewis
changed the title
gmlewis
left a comment
gmlewis
left a comment
There was a problem hiding this comment.
Also, it looks like a number of useful comments were removed from metadata.go for no apparent reason. Please review and revert the comment removals from this file.
Sorry about that, it should look good now I hope. |
gmlewis
left a comment
gmlewis
left a comment
There was a problem hiding this comment.
Better, but some lines were still unnecessarily deleted. If you review your changes in the GitHub user interface, you can see what needs to be fixed:
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4286 +/- ##
=======================================
Coverage 97.49% 97.49%
=======================================
Files 192 192
Lines 19256 19256
=======================================
Hits 18774 18774
Misses 267 267
Partials 215 215 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
yup, the current commit should fix it. |
gmlewis
left a comment
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @Skywalkr-dev!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra - @munlicode
| hasCustomDeprecated := false | ||
| for _, comment := range group.List { | ||
| if anyDeprecatedRE.MatchString(comment.Text) && !deprecatedRE.MatchString(comment.Text) { | ||
| hasCustomDeprecated = true | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
| hasCustomDeprecated := false | |
| for _, comment := range group.List { | |
| if anyDeprecatedRE.MatchString(comment.Text) && !deprecatedRE.MatchString(comment.Text) { | |
| hasCustomDeprecated = true | |
| break | |
| } | |
| } | |
| hasCustomDeprecated := slices.ContainsFunc(group.List, func(comment *ast.Comment) bool { | |
| return anyDeprecatedRE.MatchString(comment.Text) && !deprecatedRE.MatchString(comment.Text) | |
| }) |
| // add an empty line before adding doc links | ||
| isDeprecated := false | ||
| for _, op := range ops { | ||
| if op.Deprecated { | ||
| isDeprecated = true | ||
| break | ||
| } | ||
| } | ||
| if isDeprecated && !hasCustomDeprecated { | ||
| group.List = append(group.List, &ast.Comment{Text: "// Deprecated: This endpoint has been deprecated by GitHub."}) | ||
| group.List = append(group.List, &ast.Comment{Text: "//"}) | ||
| } | ||
|
|
There was a problem hiding this comment.
| // add an empty line before adding doc links | |
| isDeprecated := false | |
| for _, op := range ops { | |
| if op.Deprecated { | |
| isDeprecated = true | |
| break | |
| } | |
| } | |
| if isDeprecated && !hasCustomDeprecated { | |
| group.List = append(group.List, &ast.Comment{Text: "// Deprecated: This endpoint has been deprecated by GitHub."}) | |
| group.List = append(group.List, &ast.Comment{Text: "//"}) | |
| } | |
| isDeprecated := slices.ContainsFunc(ops, func(op *operation) bool { | |
| return op.Deprecated | |
| }) | |
| if isDeprecated && !hasCustomDeprecated { | |
| group.List = append(group.List, &ast.Comment{Text: "// Deprecated: This endpoint has been deprecated by GitHub."}, &ast.Comment{Text: "//"}) | |
| } | |
| // add an empty line before adding doc links |
| // add an empty line before adding doc links | ||
|
|
There was a problem hiding this comment.
We should keep this comment with the next line:
| // add an empty line before adding doc links | |
| // add an empty line before adding doc links |
|
@Skywalkr-dev please do not use force push for future PRs. See |
sorry about that, ill keep it in mind from next time |
|
Thank you, @Skywalkr-dev, @stevehipwell, and @alexandear! |
4 participants
Fixes : #4269