Skip to content

Adds interactive mode as default. Closes #5181#5442

Merged
waldekmastykarz merged 1 commit intopnp:mainfrom
Adam-it:set-interactive-mode-as-default
Sep 24, 2023
Merged

Adds interactive mode as default. Closes #5181#5442
waldekmastykarz merged 1 commit intopnp:mainfrom
Adam-it:set-interactive-mode-as-default

Conversation

@Adam-it
Copy link
Copy Markdown
Member

@Adam-it Adam-it commented Aug 31, 2023

🔗 Linked Issue

Closes #5181

📷 Result

After this change, every command run will automatically ask (prompt) for

  • not specified required options,
  • not specified one option among a set of required (options set)
  • when the command output will give more than one result, and only single value was expected, it will ask to pick the correct result

image

@Adam-it Adam-it mentioned this pull request Aug 31, 2023
Closed
@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Aug 31, 2023

@waldekmastykarz since last time (on the PR targeted to v7) you were going to do the review I am assigning you to this one as well 😉

@milanholemans milanholemans added the pr-major PR for the next major release label Sep 1, 2023
waldekmastykarz
waldekmastykarz requested changes Sep 9, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@waldekmastykarz waldekmastykarz left a comment

Choose a reason for hiding this comment

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

I'm getting prompts when running tests. It could be that we've added some stuff since you've submitted the PR. Could you please check the latest changes and add what's missing? I'll then review it asap to avoid further discrepancies. Thank you!

image

@waldekmastykarz waldekmastykarz marked this pull request as draft September 9, 2023 10:04
@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 9, 2023

I'm getting prompts when running tests. It could be that we've added some stuff since you've submitted the PR. Could you please check the latest changes and add what's missing? I'll then review it asap to avoid further discrepancies. Thank you!

image

That white console again🫣 🤣.
Thanks @waldekmastykarz for the recheck. I will fix it up today 👍

@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 9, 2023

@waldekmastykarz are you sure you did not miss any step? Did you clean & build (Sorry for a stupid question 😅)?

I'm asking because I:

✅ checked locally
image

✅ re-started all our PR checks
https://github.com/pnp/cli-microsoft365/actions/runs/6042314687?pr=5442

and it seems to be ok 🤔

@waldekmastykarz
Copy link
Copy Markdown
Member

waldekmastykarz commented Sep 10, 2023

Have you rebased onto latest main?

@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 10, 2023

Have you rebased onto latest main?

A ok, right. Something was merged not so long ago🤦‍♂️
No I haven't. Ok I will rebase and recheck today over the night 👍

@Adam-it Adam-it force-pushed the set-interactive-mode-as-default branch from e75bb44 to 9544506 Compare September 10, 2023 20:36
@Adam-it Adam-it marked this pull request as ready for review September 10, 2023 20:51
@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 10, 2023

ok @waldekmastykarz rebased, refixed, reready 👍

@Adam-it Adam-it marked this pull request as draft September 14, 2023 21:27
@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 14, 2023

I need to rebase and resolve conflicts 👍

@Adam-it Adam-it force-pushed the set-interactive-mode-as-default branch from 9544506 to 7d11866 Compare September 16, 2023 01:43
@Adam-it Adam-it marked this pull request as ready for review September 16, 2023 01:50
@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 16, 2023

ok @waldekmastykarz done ✅

waldekmastykarz
waldekmastykarz requested changes Sep 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@waldekmastykarz waldekmastykarz left a comment

Choose a reason for hiding this comment

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

Hey @Adam-it, when I run tests, I'm seeing prompts:

image

Could we be that we're missing some mocks due to recent changes?

@waldekmastykarz waldekmastykarz marked this pull request as draft September 23, 2023 12:05
@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 23, 2023

Hey @Adam-it, when I run tests, I'm seeing prompts:

image

Could we be that we're missing some mocks due to recent changes?

Could be 🤦‍♂️. I did last rebase 7 days ago. Since then even I merged something 😅.
I will do another round today 👍.

Sorry for that. Unfortunately this PR gets easily outdated

@waldekmastykarz
Copy link
Copy Markdown
Member

waldekmastykarz commented Sep 23, 2023

Ouch, sorry for that. If you can patch it, then I'll review it asap so that we can include it in v7

@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 23, 2023

Ouch, sorry for that. If you can patch it, then I'll review it asap so that we can include it in v7

It has to go for V7 💪.
If I will patch it today over night do you think you will have sometime tomorrow to g live it a check?

@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 23, 2023

yep a rebase is needed. As I checked this commit, recently merged, introduced two new tests that now check the validation for required options
https://github.com/pnp/cli-microsoft365/pull/5454/files#diff-6982c0e507c6aa9d6228ee767e6340ea1454f3148cdda1b931e56aaefd5fa112

@Adam-it Adam-it force-pushed the set-interactive-mode-as-default branch from 7d11866 to 8a35681 Compare September 23, 2023 20:09
@Adam-it Adam-it marked this pull request as ready for review September 23, 2023 20:22
@Adam-it
Copy link
Copy Markdown
Member Author

Adam-it commented Sep 23, 2023

ready ✅
@waldekmastykarz the stage is yours 👍

waldekmastykarz
waldekmastykarz approved these changes Sep 24, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@waldekmastykarz waldekmastykarz left a comment

Choose a reason for hiding this comment

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

Awesome! 🚀

@waldekmastykarz waldekmastykarz merged commit 8a35681 into pnp:main Sep 24, 2023
@Adam-it Adam-it deleted the set-interactive-mode-as-default branch October 11, 2023 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@waldekmastykarz waldekmastykarz waldekmastykarz approved these changes

Assignees

@waldekmastykarz waldekmastykarz

Labels

pr-major PR for the next major release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enhancement: Modify the CLI behavior to be interactive by default

3 participants

@Adam-it @waldekmastykarz @milanholemans