Refactor codebase to async/await

[martinlingstuyl]

We're currently using promise chains everywhere In the code. Refactoring to Async/await would be helpful as it gives better insight in how the code is organized, it's a less verbose and indented way to write code.

This epic is meant to track the work.

We've decided to do a max of 10 commands per PR.

• [x] #3430

Refactor promise/then to async/await.

AAD

• [x] #4933
• [x] #4936
• [x] #4938
• [x] #4942
• [x] #4944
• [x] #4946
• [x] #4948

Adaptivecard + App + Booking

• [x] #4955

Base

• [x] #4957

File

• [x] #4974

Flow

• [x] #4980
• [x] #4982

Graph

• [x] #4984

OneDrive

• [x] #4986

Outlook

• [x] #4988

PA

• [x] #5000

PP

• [x] #4992
• [x] #4994
• [x] #4990

Planner

• [x] #5002
• [x] #5003
• [x] #5004

Purview

• [x] #4996
• [x] #4998

Search + Skype + SPFx

• [x] #5010
• [x] #5152

SPO

• [x] #5011
• [x] #5015
• [x] #5023
• [x] #5022
• [x] #5021
• [x] #5020
• [x] #5042
• [x] #5043
• [x] #5044
• [x] #5045
• [x] #5046
• [x] #5047
• [x] #5048
• [x] #5049
• [x] #5050
• [x] #5051
• [x] #5097
• [x] #5098
• [x] #5099
• [x] #5100
• [x] #5101
• [x] #5102
• [x] #5103
• [x] #5104
• [x] #5105
• [x] #5106
• [x] #5107
• [x] #5108
• [x] #5109
• [x] #5110
• [x] #5111
• [x] #5112
• [x] #5113
• [x] #5114
• [x] #5115
• [x] #5116
• [x] #5117
• [x] #5118

Src files

• [ ] #5147

CLI

• [x] #5146

Auth

• [x] #5145

Util

• [x] #5144
• [x] #5079

[waldekmastykarz]

Added the needs discussion label to indicate that we need to decide how we want to split the refactoring work to allow multiple people to help out and speed up the refactoring process.

[martinlingstuyl]

Hi @pnp/cli-for-microsoft-365-maintainers, Time for some discussion, as @milanholemans PR is almost merged by @waldekmastykarz 😉.

We have like 560 commands. I was planning on splitting the work into 5 issues, each with a nice task list of 112 commands.

[Jwaegebaert]

Big fan of splitting the work!

[milanholemans]

Hi @pnp/cli-for-microsoft-365-maintainers, Time for some discussion, as @milanholemans PR is almost merged by @waldekmastykarz 😉.

We have like 560 commands. I was planning on splitting the work into 5 issues, each with a nice task list of 112 commands.

Shouldn't we divide the workload in command groups? Will be more clear if your task is just to refactor all aad, planner and pp commands for example in one go. This would also be easier to describe in the release notes. If you just split it by a fixed number of commands it is possible that for example you have to refactor all aad, planner and few pp commands. This might be a bit unclear and is harder to document in the release notes. "Refactored add, planner and some pp commands" looks a bit weird in the release notes imo 🙂

However the group spo will be a tough one to refactor in one go 🙂

[martinlingstuyl]

However the group spo will be a tough one to refactor in one go 🙂

That's why I did it like this. But let me check if I can make a more logical split.

[martinlingstuyl]

Ok, I updated it. SPO is a bit big. What do you think about splitting it in three?

[milanholemans]

Ok, I updated it. SPO is a bit big. What do you think about splitting it in three?

The proposed chunks are looking good to me. I think we can definitely split spo in smaller chunks, else no one will volunteer for that one 😄

[waldekmastykarz]

Refactoring some commands will be more work than others. My first thought was to split the work per command but that might make a mess of our issue list so perhaps we should look at slightly bigger chunks but definitely smaller than a whole workload. Also, don't underestimate the time needed to review each change and ensure there are no breaking changes in the commands behavior.

[martinlingstuyl]

Ok, so you would say split it in smaller chunks than I did? like 10-20 commands each or something?

spo is a very big task, with a lot of huge commands. But some other workloads (based on the ms graph) are relatively simple.

How about we split it per subsection?

[Adam-it]

That all are good comments and for sure we may try to divide in smaller parts in order to share the workload 👍. Just a small comment from my side is not to try to find the perfect balance and now check every comment to make perfectly equal chunks 😉. I guess I have no problem in doing something smaller then others 😅. Only joking. can't wait to try to help out 😉

[milanholemans]

After some discussion: putting this issue on hold until v6 is shipped.

[waldekmastykarz]

When we start working on it, let's split it up indeed in small chunks with a few commands each so that the workload is reasonable and people don't get stuck for weeks working on it.

[milanholemans]

v6 is shipped, are we going to proceed with this? If so, let's discuss the chunk size of the amount of commands per issue.

[waldekmastykarz]

We shouldn't have chunk sizes bigger than 10 commands. Anything more will extend the refactoring work, leading to tons of merge conflicts, and a lot of work to merge these PRs.

[Jwaegebaert]

Will we open this up for everybody? If so, it could be nice to have one chunk ready as an example. A bit like we did with adding the responses to all our command docs.

[milanholemans]

Do you mean an example PR how to refactor it? Because we have plenty of commands that use async/await already.

[waldekmastykarz]

Do you mean an example PR how to refactor it? Because we have plenty of commands that use async/await already.

In that case, let's link one as a reference for everyone to use and which we'll use when reviewing PRs.

[MathijsVerbeeck]

Hi,

I would be very interested with helping refactoring this. Could I get started on this? I'd suggest that we start with the smallerish command groups, and work up that way.

[martinlingstuyl]

Hi @MathijsVerbeeck, it would be great if you could help with this. We need to update the specs though. We've determined to take 10 commands max in every PR.

If you want to start already, just create a new issue with a checklist of 10 commands, and work through them. Reference the issue here, I'll add it to the specs.

[MathijsVerbeeck]

Perfect, thanks for the feedback @martinlingstuyl

[MathijsVerbeeck]

@martinlingstuyl I've just created the first issue + PR #4933 for the issue, #4934 for the PR

If you have any feedback regarding naming, ... shoot!

[milanholemans]

@martinlingstuyl I've just created the first issue + PR #4933 for the issue, #4934 for the PR

If you have any feedback regarding naming, ... shoot!

Added it, thx!

[martinlingstuyl]

Nice @MathijsVerbeeck!

[martinlingstuyl]

I think we should add a complete list of commands with checkboxes here, and check the ones that are PR'ed, to keep track of covered commands.

[MathijsVerbeeck]

I think we should add a complete list of commands with checkboxes here, and check the ones that are PR'ed, to keep track of covered commands.

Those are referenced in the other issue. Otherwise this one would become massive!

[martinlingstuyl]

Yes, but we need one overview, I think it will become unclear otherwise which ones have not been picked up...

[martinlingstuyl]

Yes, but we need one overview, I think it will become unclear otherwise which ones have not been picked up...

Hi @MathijsVerbeeck, looking at it, this actually looks quite neat. Let's keep it like this 👍

[nicodecleyre]

Can I help on this one?

[MathijsVerbeeck]

Can I help on this one?

I don't see any reason why not 😄 We'd just need to align who picks up which set of commands I guess, so no more me just creating issues and instantly creating the PR's 😭

I'm currently busy with the m365 pa commands. If it would be handy for you, I could give you a brief summary of what you should certainly check for when reviewing the commands.

[nicodecleyre]

I'm currently busy with the m365 pa commands. If it would be handy for you, I could give you a brief summary of what you should certainly check for when reviewing the commands.

I can start with m365 pp if that is good for you? I propose we create the issue before we start doing the work, that case we don't overlap each other and we have a clear view on who is doing what, is that ok for you?