Enhances m365 flow run get with the ability to retrieve the action details. Closes #1828

[mkm17]

Enhances m365 flow run get with the ability to retrieve the action details. Closes #1828

Hi, I have submitted this PR, but I would like to confirm some points:

• Is the deprecation message in line 150 of run-get.ts ok?
• I have introduced some interfaces instead of using 'any.' Should I extract them into a separate file?
• I am not sure how to describe the deprecated parameter in the run-get.mdx file. I understand that both parameters 'includeTriggerInformation' and 'withTrigger' should coexist.
• 'withActions' parameter can be a boolean (to include all actions) or a string with desired actions separated by a comma.
• the output of actions is similar to what is provided by the Flow API. Please let me know if this is acceptable

I noticed that there were some changes in the flows commands made by @martinlingstuyl in #5721. Once his code is merged, I can retest my changes included in this PR.

[milanholemans]

Hi @mkm17 thank you for this PR, we'll try to review it ASAP!

Is the deprecation message in line 150 of run-get.ts ok?

Technically it's ok, but if I'm not mistaken, we usually use something like:

this.warn(logger, `Option 'x' is deprecated. Please use option 'y' instead`);

I have introduced some interfaces instead of using 'any.' Should I extract them into a separate file?

According to me, it's not needed to extract it to another file if we just use it for this command. If it's used by multiple commands, we should move it to another file.

I am not sure how to describe the deprecated parameter in the run-get.mdx file. I understand that both parameters 'includeTriggerInformation' and 'withTrigger' should coexist.

Usually, we just add a deprecation message to the description. Something like:

(deprecated. Use option `x` instead) The title of the SharePoint list.

'withActions' parameter can be a boolean (to include all actions) or a string with desired actions separated by a comma.

I don't know what's agreed in the specs, let's try to stick with what was agreed there.

the output of actions is similar to what is provided by the Flow API. Please let me know if this is acceptable

I don't have the whole context of this issue, so this question is not really clear to me. Anyhow, you should always output what is returned by the API.

[Adam-it]

@mkm17 I merged @martinlingstuyl changes on the pa commands. Please give it a check and rebase with latest main before we may proceed.
thanks for you awesome work 👍 Your rock 🤩

[mkm17]

@Adam-it @milanholemans Hi I have checked my changes after updates in the pa commands and it looks ok. I have also made changes mentioned in the initial @milanholemans comment. So it is now ready to review :)

[Adam-it]

Looks really good 👏 and works perfectly 🤩
Just two small improvements we may add and we are ready to go 🚀

[mkm17]

@Adam-it I'm curious about the best approach after making a change in PR to make your job easier. Should I rebase onto the main branch and squash all my commits into one after each change?

[Adam-it]

@Adam-it I'm curious about the best approach after making a change in PR to make your job easier. Should I rebase onto the main branch and squash all my commits into one after each change?

It's not required but yes, keeping the branch up to date with main and squashing to a single commit helps 👍🙂. Thank you for asking 👍.

If you are interested more I think we tried to describe the steps here
https://pnp.github.io/cli-microsoft365/contribute/expect-during-pr#updating-the-pr-after-feedback

[mkm17]

@Adam-it Ok, I hope that I squashed and rebased it correctly :) The last process question, who should "resolve conversations" from code comments? Of course, if there is something I can do on my side to make the review process easier then let me know.

[Adam-it]

@Adam-it Ok, I hope that I squashed and rebased it correctly :) The last process question, who should "resolve conversations" from code comments? Of course, if there is something I can do on my side to make the review process easier then let me know.

If you feel like you resolved something you may mark the comment as resolved👍

[Adam-it]

thanks for the quick turnaround, we will review it ASAP

[Adam-it]

LGTM 👍
Awesome work 👏

[Adam-it]

Ready to merge 🚀

[Adam-it]

merged manually. Awesome work 👏