Skip to content

Update AD Graph Cmdlets #2734

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

Merged
merged 7 commits into from
Aug 4, 2016
Merged

Conversation

shuagarw
Copy link
Contributor

@shuagarw shuagarw commented Aug 4, 2016

Update AD Graph Cmdlets:

  • Upgrade graph sdk to 3.2.0-preview.
  • Modify exisitng create app and sp cmdlets.
  • Add new app and sp update cmdlets, key credential management cmdlets, and manage user cmdlets
  • Catch GraphErrorException in all AD cmdlets and write error with Code and Message from Graph api.

…dential management cmdlets, and manage user cmdlets
@azurecla
Copy link

azurecla commented Aug 4, 2016

Hi @shuagarw, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (shuagarw). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

/// <summary>
/// Creates a new AD application Credential.
/// </summary>
[Cmdlet(VerbsCommon.New, "AzureRmADAppCredential", DefaultParameterSetName = ParameterSet.ApplicationObjectIdWithPassword), OutputType(typeof(PSADCredential))]
Copy link
Member

Choose a reason for hiding this comment

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

SupportsShouldProcess=true

@markcowl
Copy link
Member

markcowl commented Aug 4, 2016

@shuagarw The issue is that you have removed the quotes from the csv file unintentionally while saving it. You can import the file into excel, add your columns, and save as a csv.

@shuagarw
Copy link
Contributor Author

shuagarw commented Aug 4, 2016

@markcowl Thanks!! I was looking into it and trying to find what I messed up in csv file as it is not able to parse.

@markcowl
Copy link
Member

markcowl commented Aug 4, 2016

@shuagarw if you make the SHouldProcess changes I outlined in review comments, the other static analysis error will go away. The P0 in SignautreIssues.csv is that your new Remove cmdlets have Force parameters but do not have the SupportsShouldProcess=true property in the cmdlet attribute

@shuagarw
Copy link
Contributor Author

shuagarw commented Aug 4, 2016

@markcowl
Copy link
Member

markcowl commented Aug 4, 2016

@shuagarw It looks like it was unable to find the mocks for one of your tests. This either means that the file isn't marked as CopyIfNewer or CopyAlways, or the file path is too long and you'll need to shorten the test class or test name

@cormacpayne
Copy link
Member

@shuagarw
Copy link
Contributor Author

shuagarw commented Aug 4, 2016

New on-demand after some small fixes: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/1084/

@shuagarw
Copy link
Contributor Author

shuagarw commented Aug 4, 2016

@markcowl
Copy link
Member

markcowl commented Aug 4, 2016

LGTM once the build passes

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.

4 participants