-
Notifications
You must be signed in to change notification settings - Fork 3
diff command
#27
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
diff command
#27
Conversation
|
I bet the windows failure is some kind of silly CRLF situation. |
d1f9f59 to
6a10d11
Compare
4d8e425 to
d684add
Compare
| ), | ||
| "diff should show /metrics addition:\n{}", | ||
| diff_output | ||
| ); |
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 the most interesting test.
| )?; | ||
| has_diff = true; | ||
| } | ||
| (Some(blessed), Some(local)) => { |
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.
I'm not clear on whether you can have both a blessed and local file with the same version and a non-empty diff. Is this something that we expect to happen, or is check/generate is supposed to catch it, in which case we should maybe error out here too?
|
@sunshowers I did this in a way that fits with the tools already here, but I'm curious what you think about what @davepacheco and I discussed here: how this tool might incorporate (or not) functionality like my |
Now that we're versioning the external API in omicron, it's hard to see the schema diff, because every time we change the schema, we add a whole new copy of the schema file. To see the diff, we need to diff the old file and the new file. This change does that.
To keep it orthogonal to the other commands, it assumes you have already run
generate— it doesn't rungeneratefor you. If you have made code changes but haven't generated the new schema for them, the diff will be empty.What it doesn't do (yet)
In the console I have a somewhat different script
api-diff(which doesn't take advantage of any openapi-manager machinery) that is fully GitHub-driven — it takes a PR number (or two refs) and prints the diff of either the schema or the generated TS client (more readable).I find it pretty useful to be able to see the diff for a PR without checking the PR out locally. I see openapi-manager is already git remote-aware, so maybe some version of this isn't much of a stretch. Less clear that putting the TS client diff in is appropriate, but it is an
npxone-liner to generate the client from a spec file, so it would be pretty easy.