-
Notifications
You must be signed in to change notification settings - Fork 328
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
Adds 'spe container get' command. Closes #6083 #6421
Conversation
Thank you, we'll try to review it ASAP! |
Hi @nanddeepn, just to inform you, when developing new commands, we require all new commands to use ZOD instead of the old implementation. ZOD simplifies validating options and setting up telemetry. |
@nanddeepn I added the |
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.
@nanddeepn Awesome work so far 👏👏👏
I noticed some few minor details we could sort out before we merge 👍.
Also along the way may I kindly ask you to rebase with latest main before we proceed 👍
Thank you for your awesome work. You Rock 🤩
}; | ||
|
||
try { | ||
const res = await request.get<any>(requestOptions); |
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.
can we use the ContainerProperties
interface here instead of any
?
8a40df8
to
ff8cbad
Compare
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 👍
Awesome work 👏👏
Ready to merge 🚀 |
@nanddeepn I tried to merge this one but I had a really hard time doing so due to almost 40 commits that are not part of this change 😮. |
@pnp/cli-for-microsoft-365-maintainers due to my short brake I am unassigning myself from this one so that it may be processed by someone else |
Thank you for letting me know. I will let the team know that this PR is ready to be merged and someone else will process it as in December I will be on a short break from this repo. |
Totally understand, and I appreciate your efforts so far. Enjoy your well-deserved break! |
Thanks 🤩😍. |
Adds
spe container get
command. Closes #6083