pnp
Adds the 'spfx project azuredevops pipeline add' command. Closes #5763
Nice start. Let's do some adjustments. Also, let's try to avoid using
!to suppress null checks. If we change anything, it could bite us and lead to runtime errors. If something's defined as nullable, we should check for it. If we're sure it's not null, let's update the type accordingly. Let's use TypeScript in our benefit
Thanks for the awesome feedback 🤩. On it 👍
@waldekmastykarz ready for another go?
I resolved most of the comments except two to which I added some explanations as to why it is done this way. We may think of dropping the ! and replace it with a if check. Let me know what you think.
@Adam-it, just left a comment. I also noticed that we've got some conflicts. Could you please resolve them before we continue? Thanks!
@Adam-it, just left a comment. I also noticed that we've got some conflicts. Could you please resolve them before we continue? Thanks!
I resolved the conflicts and I marked this as ready to review although I did not resolve to comments yet. I left an additional question and some info to clarify why and what? Please recheck and let me know what I am missing and how would you like me to refactor the type
@waldekmastykarz ready 👍 Please be aware I also noticed that options were not defined in ADO command as well as in the GH 🤦♂️. Sorry for that. I got it resolved along the way 👍
Added some small comments as well @Adam-it! Really nice work this! 👍💪 you rock!
Added some small comments as well @Adam-it! Really nice work this! 👍💪 you rock!
TBH I don't see any comments 😅. Did you submit your review? Or maybe over the mobile I don't see them, I will check later over the night
@martinlingstuyl I checked and I don't see any comments from you and also any review done
@martinlingstuyl thank you for your double check 👍. Except the node version I applied all of your comments.
Rechecked after last changes the flow were we use app auth type
All good 🙂
@waldekmastykarz ready for another round?
Totally! Will check asap
@waldekmastykarz is there anything I may help you with this command? I apologize for being a bit 'pushy' 🙏 but I am waiting for this to be part of the CLI to include this as a new feature in viva toolkit before v3 release (so before end of this month) 😉
Totally! Will check asap
@waldekmastykarz is there anything I may help you with this command? I apologize for being a bit 'pushy' 🙏 but I am waiting for this to be part of the CLI to include this as a new feature in viva toolkit before v3 release (so before end of this month) 😉
Sorry for the delay. If you need it asap, feel free to merge it. Since we've done one pass, we can assume the PR is in a good enough shape, and if we need to change something, we can always revisit it.