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

refactor(api, shared-data): update command-schema make target to not require version #17555

Merged
merged 2 commits into from
Feb 20, 2025

Conversation

jbleon95
Copy link
Contributor

Overview

In response to the recent command schema issues with older schema versions having newer commands added, the command-schema Make target and corresponding generate_command_schema.py script have been updated as to not need a version be manually entered. Instead, the script will automatically detect the most recent command schema version in shared-data and automatically update and overwrite that command schema JSON file. This way, as long as a new command schema is made when a chore release branch is made, there is no opportunity for human error when updating the command schema.

In a probably unnecessary goal to preserve backwards compatibility with the script, you can still give a version as a positional argument and by default it will still print to standard out. The version will be automatically detected if you don't provided a positional argument, and the flag --overwrite-shared-data needs to be included for the file to be overwritten. This way, any outside source relying on this script will still function the same way.

Test Plan and Hands on Testing

Changed some protocol engine commands and ensured that the new make target correctly modifies the most recent command schema

Changelog

  • Updated generate_command_schema.py to allow automatic detection of most recent command schema version and write to the JSON file in python
  • Updated the Makefile target to use new argument and not need a version to be manually entered
  • Fixed the .PHONY label missing the initial period

Review requests

While the generate_command_schema.py maintains backwards compatibility, I've removed the ability to target a non-most recent version using make command-schema. You can still do this by calling the script manually, but if we want to keep that functionality from the make target please let your voice be heard.

Risk assessment

Low, not touching any production code.

@jbleon95 jbleon95 requested a review from a team as a code owner February 19, 2025 19:13
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

I haven’t been following the full details of The Incident, but does it help to automatically overwrite the latest version like this? We wouldn’t want to do something like auto-create latest+1 instead? Maybe also labeling it as a prerelease draft?

@jbleon95
Copy link
Contributor Author

jbleon95 commented Feb 19, 2025

@SyntaxColoring I don't think that functionality is necessary if we keep to the following chore release branch creation protocol:

  1. A chore release branch is created that contains the latest command schema version. As soon as this branch is created, another separate branch and PR targeting edge is created that copies the latest schema and bumps it up to latest+1 (maybe at some point this can be automated)
  2. Now chore_release has the "latest" while edge has "latest+1". Any changes made in the chore release branch will change "latest" while those on edge will target "latest+1". Mergeback commits and automated tests will ensure any changes made in the chore release branch will be reflected in "latest+1" as well. Eventually the release will happen and "latest" will reflect the released command schema, will "latest+1" will contain that plus any work done since the chore release was branched off.
  3. A new chore release branch is created, rinse and repeat from the first step

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Excellent, thanks!

@jbleon95 jbleon95 merged commit 98e385a into edge Feb 20, 2025
30 checks passed
@jbleon95 jbleon95 deleted the update_generate_command_schema_script branch February 20, 2025 18:24
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.

3 participants