-
Notifications
You must be signed in to change notification settings - Fork 347
[WIP] Migrate 'entra administrativeunit remove' to Zod #6813
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
base: main
Are you sure you want to change the base?
Conversation
I see you don't have access to the git commits that are referenced in the prompt file. I told you to stop working if that happens. Try again and don't use shallow clone to make sure you can get the commit details. |
I see you don't have access to the git commits that are referenced in the |
Co-authored-by: martinlingstuyl <5267487+martinlingstuyl@users.noreply.github.com>
I've successfully accessed the reference commits after fetching the full git history. The migration has been completed using the patterns from commit 824c1eb. Changes committed in 779f911. |
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.
In the spec file I'm also missing commandOptionsSchema.parse
function calls on tests where command.action
is invoked with an options object.
force: !!args.options.force | ||
public getRefinedSchema(schema: typeof options): z.ZodEffects<any> | undefined { | ||
return schema | ||
.refine(options => options.id || options.displayName, { |
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.
It should also throw an error if both options are used at the same time.
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.
Added mutual exclusion validation using an additional refine constraint that prevents both id
and displayName
from being specified together. Also added corresponding test case. Changes committed in 72fda67.
Co-authored-by: martinlingstuyl <5267487+martinlingstuyl@users.noreply.github.com>
Added |
Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.
Original issue description:
Fixes #6812.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.