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.

Sign up for GitHub

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

Jump to bottom

[NetAppFiles]Add 2021-10-01 changes #17767

Merged
merged 22 commits into from
May 6, 2022
Merged

[NetAppFiles]Add 2021-10-01 changes #17767

merged 22 commits into from
May 6, 2022

Conversation

audunn
Copy link
Member

@audunn audunn commented Apr 11, 2022 •
edited
Loading

Description

This updates the NetApp Files module to support features in api-version 2021-10-01

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:

@isra-fel isra-fel self-assigned this Apr 13, 2022
@audunn
Copy link
Member Author

audunn commented Apr 26, 2022 •
edited
Loading

note the parameter name Cifs that static analysis is complaining about is the name of a network filesystem protocol not the plural form of anything. Is it ok to add it to tools/StaticAnalysis/Exceptions/Az.NetAppFiles/SignatureIssues.csv

@isra-fel isra-fel assigned wyunchi-ms and unassigned isra-fel Apr 27, 2022
@audunn
Copy link
Member Author

audunn commented Apr 27, 2022

@wyunchi-ms cmdLet review approved, are we ready to move on with this now ?

BethanyZhou
BethanyZhou reviewed Apr 28, 2022
View reviewed changes
Copy link
Contributor

@BethanyZhou BethanyZhou left a comment

Choose a reason for hiding this comment

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

Please check whether there are some place holder in help MDs and fill them.

src/NetAppFiles/NetAppFiles/Az.NetAppFiles.psd1 Outdated Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/ChangeLog.md Outdated Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/Snapshot/RestoreFilesNetAppFilesSnapshot.cs Outdated Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/Subvolume/RemoveNetAppFilesSubvolume.cs Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/Volume/NewNetAppFilesVolume.cs Outdated Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/help/Get-AzNetAppFilesSubvolume.md Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/help/Get-AzNetAppFilesSubvolume.md Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/help/Get-AzNetAppFilesSubvolumeMetadata.md Outdated Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/help/Get-AzNetAppFilesSubvolumeMetadata.md Outdated Show resolved Hide resolved
src/NetAppFiles/NetAppFiles/help/Get-AzNetAppFilesSubvolumeMetadata.md Outdated Show resolved Hide resolved
@audunn
Copy link
Member Author

audunn commented Apr 28, 2022

@BethanyZhou pr updated based on comments could you please check again?

BethanyZhou
BethanyZhou reviewed Apr 29, 2022
View reviewed changes
Copy link
Contributor

@BethanyZhou BethanyZhou left a comment

Choose a reason for hiding this comment

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

Looks good to me. Besides, please check whether there is any reference link can be added in RELATED LINKS section for each command.

src/NetAppFiles/NetAppFiles/help/Restore-AzNetAppFilesSnapshotFile.md Outdated Show resolved Hide resolved
@audunn
Copy link
Member Author

audunn commented Apr 29, 2022

@BethanyZhou
Should we perhaps add NetAppFiles to New-AzExportPolicyObject so that it is New-AzNetAppFilesExportPolicyObject ?

@BethanyZhou
Copy link
Contributor

@BethanyZhou Should we perhaps add NetAppFiles to New-AzExportPolicyObject so that it is New-AzNetAppFilesExportPolicyObject ?

Yes, that would be better to name cmdlet with pattern "Verb-Az{ModuleName}{Noun}"

@audunn
Copy link
Member Author

audunn commented May 2, 2022

@BethanyZhou PR updated

@audunn
Copy link
Member Author

audunn commented May 3, 2022

@BethanyZhou @wyunchi-ms can we move this along now?

@audunn
Copy link
Member Author

audunn commented May 4, 2022

@BethanyZhou @wyunchi-ms can we move this along now?

BethanyZhou
BethanyZhou reviewed May 5, 2022
View reviewed changes
Copy link
Contributor

@BethanyZhou BethanyZhou left a comment

Choose a reason for hiding this comment

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

Generally looks good to me except one comment.

src/NetAppFiles/NetAppFiles/help/Get-AzNetAppFilesVault.md Outdated Show resolved Hide resolved
@BethanyZhou
Copy link
Contributor

/azp run azure-powershell - security-tools

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

BethanyZhou
BethanyZhou approved these changes May 6, 2022
View reviewed changes
Copy link
Contributor

@BethanyZhou BethanyZhou left a comment

Choose a reason for hiding this comment

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

Looks good to me

@BethanyZhou BethanyZhou merged commit 2086a99 into Azure:main May 6, 2022
@isra-fel isra-fel mentioned this pull request May 26, 2022
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BethanyZhou BethanyZhou BethanyZhou approved these changes

Assignees

@wyunchi-ms wyunchi-ms

Labels
Cmdlet Review Complete Waiting for CI :shipit:
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@audunn @BethanyZhou @isra-fel @wyunchi-ms