-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP-311382: Update foas changelog metadata
command to support upcoming
#748
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
Conversation
// Validate that the API version use the correct date format YYYY-MM-DD | ||
for _, version := range o.versions { | ||
// Upcoming version has the format YYYY-MM-DD.upcoming, here we remove .upcoming to validate the date format. | ||
version = strings.ReplaceAll(version, ".upcoming", "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is where I suggested to simply try to use apiversion.go
and create a new version, throwing an error if it's invalid, that way you don't need to maintain two different logics to parse versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good point. I updated the logic to reuse apiversion.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks for the changes
opts := &Opts{ | ||
specRevision: "test", | ||
runDate: "2024-01-01", | ||
versions: []string{"2024-01-01.upcoming", "2025-01-01"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: is there chance for us to migrate from string to object with multiple keys in the future?
Not sure how much involved it would be, asking curious question.
Thinking of all code generation tools we need to invent some parser for custom date format and prefixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have already the version struct here. You can create a new version struct from a string with this piece of code: apiVersion, err := apiversion.New(apiversion.WithVersion(version));
. I am updating foascli to use always this struct where possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andreaangiolillo Thank you. Awesome news! Unification can help platform and integrations team as well as we see questions to support stability levels through SDK + other generation methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear - would format of the versions.json in openapi/sdk change as well?
Here is code that does fetch that file: https://github.com/mongodb/atlas-sdk-go/blob/main/tools/scripts/fetch.sh#L29
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it won't change. It will still be a JSON array. You will start to see upcoming entries like this one:
[
"2023-01-01",
"2023-02-01",
"2023-10-01",
"2023-11-15",
"2024-05-30",
"2024-08-05",
"2024-10-23",
"2024-11-13",
"2025-02-19",
"2025-03-12"
"2025-03-12.upcoming"
]
There was an issue last week on the dev SDK about this change and was fixed: https://mongodb.slack.com/archives/CHEULRC9W/p1749200548092979?thread_ts=1749199877.955329&cid=CHEULRC9W
Proposed changes
CLOUDP-311382
This PR proposes the following changes:
foas changelog metadata create
to consider upcoming API version when generating the metadata filefoas changelog create
to skip upcoming api version when generating the changelog. The support will be added in CLOUDP-315486Next
I will test the changes once the PR is merged by running the FOAS release. I may have to create follow up PRs to fix the release flow.