Fix usage of Resolve-RepositoryElements -DisableValidation #243
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are only two legitimate instances in the module as of today where we should be disabling the validation that Resolve-RepositoryElements provides. All other instances were likely introduced due to mistaken copy/paste issues. The only reason we'd want to disable the validation is if there are multiple parametersets that need to be considered for the function, where at least one _would_ require `OwnerName`/`RepositoryName` while at least one other one _would not_. This would have meant that we would have attempted to call those GitHub endpoints with incomplete path information which would have most likely resulted in an error that we could have caught sooner without needing to invoke a web request. I've fixed up all of these incorrect instances, and added a comment to the two remaining ones to remind others later on why that switch is being used, and help prevent incorrect usage in the future.
|
/azp run PowerShellForGitHub-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run PowerShellForGitHub-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
There are only four legitimate instances in the module as of today where we should be disabling the validation that Resolve-RepositoryElements provides. All other instances were likely introduced due to mistaken copy/paste issues.
The only reason we'd want to disable the validation is if there are multiple parametersets that need to be considered for the function, where at least one would require
OwnerName/RepositoryNamewhile at least one other one would not.This would have meant that we would have attempted to call those GitHub endpoints with incomplete path information which would have most likely resulted in an error that we could have caught sooner without needing to invoke a web request.
I've fixed up all of these incorrect instances, and added a comment to the two remaining ones to remind others later on why that switch is being used, and help prevent incorrect usage in the future.
I've also cleaned up unnecessary passing of
BoundParameterssince it already does the correct thing by default.Issues Fixed
None
References
n/a
Checklist
Comment-based help added/updated, including examples.New/changed code adheres to our coding guidelines.New/changed code continues to support the pipeline.Changes to the manifest file follow the manifest guidance.Unit tests were added/updated and are all passing. See testing guidelines. This includes making sure thatall pipeline input variations have been covered.Relevant usage examples have been added/updated in USAGE.md.If desired, ensure your name is added to our Contributors list