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: annotate each path with other release versions #89

Closed
wants to merge 0 commits into from

Conversation

cmars
Copy link
Contributor

@cmars cmars commented Dec 5, 2021

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 #78.

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! Questions in the diff for my own edification (unless it turns out I stumbled across something missing 🙂)

resource.go Outdated
// versions affecting the path. This supports navigation across versions.
for _, rc := range resourceVersions.versions {
for path, pathInfo := range rc.Paths {
pathInfo.ExtensionProps.Extensions[ExtSnykApiReleases] = pathReleases[path].Strings()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a bit of code that's automatically adding extension sections to the spec output? I see the data added here, and I see a string representation utility function at the end of the PR, but no updates to have this put in spec, so guessing that's automatic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These extensions are basically being added on load, which gets merged into the compiled result by vervet.Merge.

The utility function is used to read the value of extensions provided in the source OpenAPI spec.

resource.go Outdated
@@ -139,31 +144,49 @@ func LoadResourceVersions(epPath string) (*ResourceVersions, error) {
}

func LoadResourceVersionsFileset(specYamls []string) (*ResourceVersions, error) {
var eps ResourceVersions
var resourceVersions ResourceVersions
Copy link
Contributor

Choose a reason for hiding this comment

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

Good renaming; what was eps for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Endpoints.

Copy link
Contributor Author

@cmars cmars left a comment

Choose a reason for hiding this comment

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

Replies to comments.

resource.go Outdated
@@ -139,31 +144,49 @@ func LoadResourceVersions(epPath string) (*ResourceVersions, error) {
}

func LoadResourceVersionsFileset(specYamls []string) (*ResourceVersions, error) {
var eps ResourceVersions
var resourceVersions ResourceVersions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Endpoints.

resource.go Outdated
// versions affecting the path. This supports navigation across versions.
for _, rc := range resourceVersions.versions {
for path, pathInfo := range rc.Paths {
pathInfo.ExtensionProps.Extensions[ExtSnykApiReleases] = pathReleases[path].Strings()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These extensions are basically being added on load, which gets merged into the compiled result by vervet.Merge.

The utility function is used to read the value of extensions provided in the source OpenAPI spec.

@cmars cmars closed this Dec 7, 2021
@genslein genslein deleted the feat/path-change-versions branch May 10, 2022 15:06
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