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

feat: add version deprecation and sunset eligibility #90

Merged
merged 2 commits into from
Dec 6, 2021

Conversation

cmars
Copy link
Contributor

@cmars cmars commented Dec 6, 2021

Annotate paths in a given resource version with the newer release
version that deprecates it, and when the deprecated version may be
sunset.

This may be used in linting, middleware, and documentation.

Implements #86.

Stacked on #89.

@cmars cmars requested a review from a team as a code owner December 6, 2021 00:07
Copy link
Contributor

@jcsackett jcsackett left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! One question in the diff below, more a question of style than any sort of blocker--take it or leave it 🤷🏼 🙂

version.go Outdated
// DeprecatedBy returns true if the given version deprecates the caller target
// version.
func (v *Version) DeprecatedBy(vr *Version) bool {
return v.Compare(vr) < 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do < 0 here? The returns from Compare are only -1, 0, or 1, so == -1 makes more sense to me to show it's a particular value.

This is fine by me if your preference is to stick w/ < -- I'm curious if we would ever add any other values coming from Compare?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We wouldn't, a direct == -1 would suffice here. Looking at this more carefully now, I'm now wondering, does a version deprecate itself? I'm thinking it probably shouldn't!

API documentation needs to represent a "changelog" per resource
endpoint: a list of versions in which a change to a given resource
endpoint was released.

This change adds a list of other resource release versions in which a
particular path was declared for such navigation purposes.

Implements snyk#78.
Annotate paths in a given resource version with the newer release
version that deprecates it, and when the deprecated version may be
sunset.

This may be used in linting, middleware, and documentation.

Implements snyk#86.
@cmars cmars merged commit c43ee26 into snyk:main Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants