Skip to content

Fixes for param validation, forwarding, and exit behavior - #265

Merged
Josh Wittner (jwittner) merged 2 commits into
microsoft:developfrom
jwittner:user/jowitt/guidAndExitFixes
Aug 5, 2024
Merged

Fixes for param validation, forwarding, and exit behavior#265
Josh Wittner (jwittner) merged 2 commits into
microsoft:developfrom
jwittner:user/jowitt/guidAndExitFixes

Conversation

@jwittner

Copy link
Copy Markdown
Member
  • Fixes for AzureSubscription to support use of guid
  • Converted PATLifetime and SearchDepth to uint to enforce valid range
  • Force ProjectManifestPath or SearchPath or both to be provided via parameter set name groupings.
  • Only allow SearchDepth if SearchPath is provided.
  • Validate ProjectManifestPath is a valid file in a ValidateScript attribute.
  • Do not use 'exit' but thrown an exception instead - exit works ok from scripts, but not well from cmdlets.
  • Fix bug where ProjectManifestPath was used as the search path if SearchPath was provided. SearchPath value was previously unused...
  • Move Az module validation into function that requires it so other usage of UnitySetup is not blocked by it.

[Parameter(Mandatory=$false, ParameterSetName = "SearchPath")]
[Parameter(Mandatory=$false, ParameterSetName = "SearchPathAndProjectManifest")]
[uint]$SearchDepth = 3
)

@Exolun Michael Smith (Exolun) Aug 5, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with these validation attributes. Since we're moving towards the use of either a $ProjectManifestPath OR $SearchPath + $SearchDepth I am assuming that you can create different validation rules for different Parameter sets?

Does this set of rules express that if $ProjectManifest is provided, then $SearchPath and $SearchDepth become mandatory (depth optional), while if the search parameters are provided, the manifest parameter must be null?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently what I have requires that you specify either ProjectManifestPath or SearchPath or both, and it ensures that you cannot pass SearchDepth unless SearchPath is provided. The logic seemed to be that both was allowed, but we can make it be strict 'or' if that's the behavior we want.

@Exolun Michael Smith (Exolun) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Look like good fixes, I think these are okay to merge - I have a test script that I use to validate changes, but given the new parameterset validations I think I will also test some possible parameter combinations.

@jwittner
Josh Wittner (jwittner) merged commit fa32f13 into microsoft:develop Aug 5, 2024
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