Skip to content

Remove #requires -pssnapin#19320

Merged
daxian-dbw merged 2 commits intoPowerShell:masterfrom
SteveL-MSFT:requires-pssnapin
Apr 3, 2023
Merged

Remove #requires -pssnapin#19320
daxian-dbw merged 2 commits intoPowerShell:masterfrom
SteveL-MSFT:requires-pssnapin

Conversation

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Mar 13, 2023 •
edited
Loading

PR Summary

Removing unsupported code

PR Context

Fix #7412

PR Checklist

@SteveL-MSFT SteveL-MSFT requested a review from daxian-dbw as a code owner March 13, 2023 21:08
@ghost ghost assigned anmenaga Mar 13, 2023
@SteveL-MSFT SteveL-MSFT mentioned this pull request Mar 13, 2023
Closed
3 tasks
daxian-dbw
daxian-dbw reviewed Mar 14, 2023
View reviewed changes
@pull-request-quantifier-deprecated
Copy link

pull-request-quantifier-deprecated bot commented Mar 20, 2023

This PR has 67 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details 

Label      : Small
Size       : +8 -59
Percentile : 26.8%

Total files changed: 4

Change summary by file extension:
.cs : +8 -59

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@ghost ghost added the Review - Needed The PR is being reviewed label Mar 28, 2023
@ghost
Copy link

ghost commented Mar 28, 2023

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

daxian-dbw
daxian-dbw reviewed Apr 3, 2023
View reviewed changes
src/System.Management.Automation/engine/parser/ast.cs
/// <code>#requires -PSSnapin Snapin -Version 2</code>
/// If no snapins are required, this property is an empty collection.
/// </summary>
public ReadOnlyCollection<PSSnapInSpecification> RequiresPSSnapIns { get; internal set; }
Copy link
Member

@daxian-dbw daxian-dbw Apr 3, 2023

Choose a reason for hiding this comment

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

This is a public API, so removing it is a breaking change. Would it be acceptable?

Copy link
Member

@daxian-dbw daxian-dbw Apr 3, 2023

Choose a reason for hiding this comment

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

I don't find anything on GitHub using RequiresPSSnapIns except for PowerShell repo itself:RequiresPSSnapIns - grep.app

So I think it's OK to make this breaking change

@daxian-dbw daxian-dbw added Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log and removed Review - Needed The PR is being reviewed labels Apr 3, 2023
@daxian-dbw daxian-dbw merged commit 7c0fe76 into PowerShell:master Apr 3, 2023
CarloToso pushed a commit to CarloToso/PowerShell that referenced this pull request Apr 8, 2023
@stevenebutler
Pr/19379 (#1)
1a6707d 
* Remove FormObject.cs and FormObjectCollection.cs (PowerShell#19383)

* Exclude redundant parameter aliases from completion results (PowerShell#19382)

* Revert "Remove FormObject.cs and FormObjectCollection.cs (PowerShell#19383)" (PowerShell#19387)

This reverts commit 190c99a.

* Add the parameter `-RelativeBasePath` to `Resolve-Path` (PowerShell#19358)

* Fix a crash in the type inference code (PowerShell#19400)

* Remove GetResponseObject (PowerShell#19380)

* Add `-Environment` parameter to `Start-Process` (PowerShell#19374)

* Add `-Environment` parameter to `Start-Process`

* address codefactor

* fix test for Windows

* handle case where value is $null to remove env var

* change variables to make it more clear what the test is doing

* Add PoolNames variable group to compliance pipeline (PowerShell#19408)

* Improve package management acceptance tests by not going to the gallery (PowerShell#19412)

* Skip VT100 tests on Windows Server 2012R2 as console does not support it (PowerShell#19413)

* Update the `ICommandPredictor` interface to reduce boilerplate code from predictor implementation (PowerShell#19414)

* Enable type conversion of `AutomationNull` to `$null` for assignment (PowerShell#19415)

* Remove code related to `#requires -pssnapin` (PowerShell#19320)

* Support CTRL-C when reading data and connection hangs for `Invoke-RestMethod` and `Invoke-WebRequest` (PowerShell#19330)

* Update to the latest NOTICES file (PowerShell#19332)

* Update the cgmanifest (PowerShell#19459)

* WIP: Harden default command test. (PowerShell#19416)
@ghost
Copy link

ghost commented Apr 20, 2023

🎉v7.4.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Apr 20, 2023
Closed
@diecknet diecknet mentioned this pull request Nov 9, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

Assignees

@anmenaga anmenaga

Labels

Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log Small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Should we remove the -pssnapin parameter for #requires statement?

3 participants

@SteveL-MSFT @daxian-dbw @anmenaga