Feature : Added sunset date for changelog generation at Endpoint and Property level.#795
Feature : Added sunset date for changelog generation at Endpoint and Property level.#795jainsachi88 wants to merge 10 commits intooasdiff:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #795 +/- ##
=======================================
Coverage 89.55% 89.55%
=======================================
Files 239 239
Lines 12154 12154
=======================================
Hits 10884 10884
Misses 840 840
Partials 430 430
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| api-sunset-date-too-small: "sunset date %s is too small, must be at least %s days from now" | ||
| endpoint-added: endpoint added | ||
| endpoint-deprecated: endpoint deprecated | ||
| endpoint-deprecated: "endpoint deprecated%.0s" |
There was a problem hiding this comment.
what will %.0s show ? should there be a space ?
There was a problem hiding this comment.
"%.0s" acts like a placeholder that consumes an argument but prints nothing.
We used this in the changelog to keep a consistent message format like "endpoint deprecated" without adding extra text or spaces.
No space is needed here, because we don't want a trailing space in the message.
There was a problem hiding this comment.
if it prints nothing, then why have it ? Can you not just leave the message as it is, "endpoint deprecated" ?
There was a problem hiding this comment.
Yes i have removed that "%.0s" and updated the code.
reuvenharrison
left a comment
There was a problem hiding this comment.
Review: Critical Bug — %!(EXTRA ...) garbage in output
Problem
The PR adds date to the Args slice of NewApiChange() calls, but the message templates in messages.yaml are not updated to consume the extra argument. The localizer uses fmt.Sprintf(pattern, args...) (localizer.go:28), so extra arguments produce Go's %!(EXTRA ...) formatting artifact in the output.
EndpointDeprecatedId — template: "endpoint deprecated" (zero %s placeholders)
- Before:
Args = nil→"endpoint deprecated" - After:
Args = []any{date}→"endpoint deprecated%!(EXTRA string='9999-08-10')"
RequestPropertyDeprecatedId — template: "request property %s deprecated" (one %s)
- Before:
Args = []any{propName}→"request property 'propName' deprecated" - After:
Args = []any{propName, date}→"request property 'propName' deprecated%!(EXTRA string='2027-02-06')"
Same issue for ResponsePropertyDeprecatedId.
Sunset date is already in the output
The current code already includes the sunset date via the Details mechanism. All three call sites use .WithDetails(formatDeprecationDetailsWithSunset(date, op.Extensions)), which appends (sunset: 2027-02-06) through getDetailsSuffix(). So passing date in Args is redundant.
Tests weakened to mask the bug
The exact-match assertions (require.Equal) were replaced with substring checks (require.Contains), which hide the %!(EXTRA ...) garbage. The original exact-match tests would have caught this.
messages.yaml not actually modified
The PR description lists messages.yaml as modified, but the diff shows no changes to it — only the auto-generated localizations.go timestamp changed.
Suggested fix
Either:
- Update the message templates in
messages.yamlto include the date placeholder and stop usingWithDetails()for the same information, or - Leave the code as-is — the sunset date already appears correctly via the
Detailsmechanism.
Feature : Added sunset date for changelog generation at Endpoint and Property level.
Files Modified:
checker/check_api_deprecation.go
checker/check_request_property_deprecation.go
checker/check_response_property_deprecation.go
checker/localizations/localizations.go
checker/localizations_src/en/messages.yaml
Output :
"Endpoint deprecated with sunset date '2027-02-06'"
"Response property 'DeployTagValue' deprecated with sunset date '2027-02-06'"