Skip to content
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

Adding VivaConnectionsDefaultStart option to SetHomeSite #2779

Merged
merged 11 commits into from
Feb 7, 2023
Merged

Adding VivaConnectionsDefaultStart option to SetHomeSite #2779

merged 11 commits into from
Feb 7, 2023

Conversation

reshmee011
Copy link
Contributor

Before creating a pull request, make sure that you have read the contribution file located at

https://github.com/pnp/powerShell/blob/dev/CONTRIBUTING.md

Type

  • New Feature

What is in this Pull Request ?

Adding ability to set VivaConnectionsDefaultStart

@gautamdsheth
Copy link
Collaborator

Awesome stuff @reshmee011 , just 2 comments :)

  1. Can you please move the executequery outside if/else condition like this ?
if(){
// code
}
else{
// more code
}

ClientContext.ExecuteQueryRetry();
  1. Can you also please update the related docs for this ? It is located in docs folder (docs > set-pnphomesite.md)

Also, @anoopt , dont work on this now 🤣😭

@reshmee011
Copy link
Contributor Author

@gautamdsheth : thanks for the prompt review, I need to remember about updating docs in the future. @anoopt : hope you have not worked on it yet. @gautamdsheth : for any new features, shall I raise it as an issue first before I start working on it?

@gautamdsheth
Copy link
Collaborator

Hi @reshmee011 , any reason you changed the parameter to bool ? I liked the SwitchParameter :)

Maybe we can keep using the SwitchParameter ?

No, no need to open an issue for this. It was just something we discussed while chatting about something totally different !

@reshmee011
Copy link
Contributor Author

@gautamdsheth , bool will allow to pass $true or $false to toggle on and off the viva experience to the Home Site, switch parameter does not give that ability.

@gautamdsheth
Copy link
Collaborator

We can set the value when using SwitchParameter 😊

Like this:

-VivaConnectionsDefaultStart ---> sets value to true

or

-VivaConnectionsDefaultStart:$false ---> sets value to false

or 

-VivaConnectionsDefaultStart:$true ---> set value to true. Not necessarily needed because if the parameter is specified, then it is true.

You can check this example:

https://pnp.github.io/powershell/cmdlets/Enable-PnPPowerShellTelemetry.html

We are using -Force as a switch parameter.

@reshmee011
Copy link
Contributor Author

@gautamdsheth : Awesome, thanks for the example how to use the switchparameter to turn on and off. I did not think about it. Happy to keep it as switch parameter. Shall I make updates to the code and docs to revert the VivaConnectionsDefaultStart to switch parameter. ?

@gautamdsheth
Copy link
Collaborator

Yes please 😊

@reshmee011
Copy link
Contributor Author

@gautamdsheth : I have reverted the parameter to be switch parameter and amended the get-pnphomesite to return its value with an additional parameter.

@gautamdsheth gautamdsheth merged commit bb4328d into pnp:dev Feb 7, 2023
@gautamdsheth
Copy link
Collaborator

Thank you, merged it !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants