Skip to content

Implement -PassThru for all Set-* cmdlets #217

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

Closed
TylerLeonhardt opened this issue Jun 2, 2020 · 4 comments · Fixed by #276
Closed

Implement -PassThru for all Set-* cmdlets #217

TylerLeonhardt opened this issue Jun 2, 2020 · 4 comments · Fixed by #276
Labels
breaking change This includes a change in existing published module behavior. discussion We are looking for additional community feedback on this topic before proceeding further. enhancement An issue or pull request introducing new functionality to the project.

Comments

@TylerLeonhardt
Copy link
Member

Feature Idea Summary

It's usually good practice for Set-* cmdlets to output nothing and then offer a -PassThru to output the result.

This allows output to not be as noisy and so that you don't have to pipe to Out-Null

Feature Idea Additional Details

All this would mean is:

  • Adding a -PassThru parameter to all Set-* functions
  • do something like this:
$result = Invoke-GHRestMethod @params
if ($PSBoundParameters.ContainsKey('PassThru'))
{
    $result
}

Requested Assignment

I'm just suggesting this idea, at this time. I'm not planning on implementing it.

@TylerLeonhardt TylerLeonhardt added enhancement An issue or pull request introducing new functionality to the project. triage needed An issue that needs to be reviewed by a member of the team. labels Jun 2, 2020
@HowardWolosky
Copy link
Contributor

Actually, for your sample code, you'd need it to be:

$result = Invoke-GHRestMethod @params
if ($PassThru)
{
    return $result
}

The way you wrote it, you'd still behave using PassThru even if someone supplied -PassThru:$false.

@HowardWolosky
Copy link
Contributor

This is an interesting idea. I think I like it. The main issue with it is that it would be a breaking change for users. There are a lot of breaking changes lately, and I'm trying to be conscious about that. We just added a breaking change with the Remove-* functions due to #171. And #192 is deprecating the Private switch in favor of the Visible parameter.

I think that this could be done in a phased approach.

Phase 1: Add the PassThru switch and include the behavior above, but still return the result with a warning indicating that in a future release, the result will only be returned if the PassThru switch is provided.
Phase 2: When that "future release" time is coming, check-in a change that removes the warning and only returns the result when the switch is provided.

Leaving this open for discussion to hear other people's thoughts on this Issue's suggestion in general, as well as the phased approach due to the breaking change.

@HowardWolosky HowardWolosky added discussion We are looking for additional community feedback on this topic before proceeding further. and removed triage needed An issue that needs to be reviewed by a member of the team. labels Jun 2, 2020
@TylerLeonhardt
Copy link
Member Author

TylerLeonhardt commented Jun 2, 2020

Oh yeah sorry it should be:

$result = Invoke-GHRestMethod @params
if ($PassThru.IsPresent)
{
    return $result
}

since it's a switch parameter... that way it works in PowerShell strict mode lol

@HowardWolosky HowardWolosky added the breaking change This includes a change in existing published module behavior. label Jun 2, 2020
HowardWolosky added a commit to HowardWolosky/PowerShellForGitHub that referenced this issue Aug 13, 2020
…ssThru)

All state-changing commands (with the exception of New-*) are now
silent by default.  Users can pass-in `-PassThru` to make them return
the result that used to be returned by default.

Users can revert back to the previous behavior by leveraging the new
configuration value: DefaultPassThru.

Resolves microsoft#217
@HowardWolosky
Copy link
Contributor

Decided to just implement this anyway, since the next release is already going to be full of breaking changes.

HowardWolosky added a commit to HowardWolosky/PowerShellForGitHub that referenced this issue Aug 13, 2020
…ssThru)

All state-changing commands (with the exception of New-*) are now
silent by default.  Users can pass-in `-PassThru` to make them return
the result that used to be returned by default.

Users can revert back to the previous behavior by leveraging the new
configuration value: DefaultPassThru.

Resolves microsoft#217
HowardWolosky added a commit that referenced this issue Aug 14, 2020
…ssThru) (#276)

All state-changing commands (with the exception of `New-*`) are now silent by default.  Users can pass-in `-PassThru` to make them return the result that used to be returned by default.

Users can revert back to the previous behavior by leveraging the new configuration value: `DefaultPassThru`.

Resolves #217
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This includes a change in existing published module behavior. discussion We are looking for additional community feedback on this topic before proceeding further. enhancement An issue or pull request introducing new functionality to the project.
Projects
None yet
2 participants