GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Use `is not` syntax and remove unnecessary parentheses by xtqqczze · Pull Request #13323 · PowerShell/PowerShell

Conversation

@xtqqczze

@xtqqczze xtqqczze commented Jul 30, 2020

Copy link
Copy Markdown
Contributor

PR Summary

Instead of the huge changes in #13286, a more modest change using is not syntax to remove parentheses.

PR Context

#13286

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
  • Breaking changes
    • None
    • OR
    • Experimental feature(s) needed
      • Experimental feature name(s):
  • User-facing changes
    • Not Applicable
    • OR
    • Documentation needed
      • Issue filed:
  • Testing - New and feature
    • N/A or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
  • Tooling
    • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
    • OR
    • I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
      • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
      • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
      • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
      • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

daxian-dbw and vexx32 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@xtqqczze
Use is not syntax
bf0f1ea
-replace '\(!\(([^)]+) is ([^)]+)\)\)', '($1 is not $2)'
@ghost ghost assigned Dongbo Wang (daxian-dbw) Jul 30, 2020
@xtqqczze
xtqqczze marked this pull request as ready for review July 30, 2020 23:41
@xtqqczze
xtqqczze requested review from Aditya Patwardhan (adityapatwardhan), Andrew (anmenaga) and Dongbo Wang (daxian-dbw) as code owners July 30, 2020 23:41
@xtqqczze xtqqczze mentioned this pull request Jul 30, 2020
Use is not null syntax #13286
Closed
8 of 14 tasks
@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

Dongbo Wang (@daxian-dbw) Can you restart CI?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

It appears Codacy (SonarCSharp) does not understand C# 9 pattern matching.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rjmholt

Rob Holt (rjmholt) commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

/azp help

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@azure-pipelines

azure-pipelines Bot commented Jul 31, 2020

Copy link
Copy Markdown
Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rjmholt

Rob Holt (rjmholt) commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

/azp list

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@azure-pipelines

azure-pipelines Bot commented Jul 31, 2020

Copy link
Copy Markdown
CI/CD Pipelines for this repository:
  • PowerShell-CI-macos
  • PowerShell-CI-linux
  • PowerShell-CI-windows
  • PowerShell-CI-static-analysis
  • PowerShell-CI-install-ps
  • PowerShell-CI-SSH
  • SSH-Tests
  • PowerShell-Windows-Packaging-CI
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rjmholt

Rob Holt (rjmholt) commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

It looks like the same test is failing across all platforms, implying a regression. In that case, I think it's safer for us not to take this change

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

/azp run

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@azure-pipelines

azure-pipelines Bot commented Jul 31, 2020

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 13323 in repo PowerShell/PowerShell
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xtqqczze
xtqqczze force-pushed the isnot branch from 76dcec1 to bf0f1ea Compare July 31, 2020 01:19
vexx32
Rain Sallow (/u/ta11ow) (vexx32) suggested changes Jul 31, 2020
View reviewed changes

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Found a couple places where the substitution mangled the original logic. Once we get those fixed we should be right to go. 🙂

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/System.Management.Automation/engine/TypeTable.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/System.Management.Automation/engine/hostifaces/ConnectionFactory.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/System.Management.Automation/engine/hostifaces/ConnectionFactory.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/System.Management.Automation/engine/parser/Compiler.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 31, 2020
daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Jul 31, 2020
View reviewed changes
Comment thread ...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionAnalysis.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@xtqqczze @vexx32
Apply suggestions from code review
bd1f4c4
Co-authored-by: Joel Sallow (/u/ta11ow) <[email protected]>
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 31, 2020
@rjmholt Rob Holt (rjmholt) added Review - Maintainer The PR/issue needs a review from the PowerShell repo Maintainers Review - Needed The PR is being reviewed labels Jul 31, 2020
xtqqczze and others added 2 commits July 31, 2020 16:35
@xtqqczze @daxian-dbw
Update src/System.Management.Automation/engine/CommandCompletion/Comp…
0e94eba
…letionAnalysis.cs

Co-authored-by: Dongbo Wang <[email protected]>
@xtqqczze
Remove unnecessary parenthesis
be3db27
vexx32
Rain Sallow (/u/ta11ow) (vexx32) approved these changes Jul 31, 2020
View reviewed changes

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this 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, thanks for sorting this out! 💖

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@xtqqczze
Use null conditional
62d12e2
@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

I have fully reviewed this PR and it is now ready to merge as far as I am concerned.

vexx32 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw
Keep the original != syntax
7ea7260
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Jul 31, 2020
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jul 31, 2020

Copy link
Copy Markdown
Member

PoshChan-Bot (@PoshChan) Please remind me in 1 hour

PoshChan reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ghost ghost removed the Review - Needed The PR is being reviewed label Jul 31, 2020
@PoshChan

PoshChan-Bot (PoshChan) commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

Dongbo Wang (@daxian-dbw), this is the reminder you requested 1 hour ago

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 4597b41 into PowerShell:master Jul 31, 2020
@daxian-dbw Dongbo Wang (daxian-dbw) added CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log and removed Review - Maintainer The PR/issue needs a review from the PowerShell repo Maintainers labels Jul 31, 2020
@daxian-dbw Dongbo Wang (daxian-dbw) added this to the 7.1.0-preview.7 milestone Jul 31, 2020
@xtqqczze
xtqqczze deleted the isnot branch August 1, 2020 16:59
@xtqqczze xtqqczze mentioned this pull request Aug 1, 2020
Fix RCS1221: Use pattern matching instead of combination of 'as' operator and null check #13333
Merged
8 tasks done
@baosj0

baosj0 commented Aug 2, 2020

Copy link
Copy Markdown

this "is not "syntax make my build failed using visualstudio2019

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@xtqqczze

xtqqczze commented Aug 2, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

baosj0 You need to use Visual Studio 2019 16.7 Preview (link), I have opened a PR to update build documentation (#13336).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Aug 2, 2020
Bump Visual Studio 2019 required version in build documention #13336
Merged
Use is not syntax part 2 #13338
Merged
@baosj0

baosj0 commented Aug 3, 2020

Copy link
Copy Markdown

baosj0 You need to use Visual Studio 2019 16.7 Preview (link), I have opened a PR to update build documentation (#13336).

yes , no problem now

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@TravisEz13 Travis Plunk (TravisEz13) modified the milestones: 7.1.0-preview.7, 7.1.0-preview.6 Aug 5, 2020
@ghost

Deleted user (ghost) commented Aug 17, 2020

Copy link
Copy Markdown

🎉v7.1.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

  • Release Notes
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@xtqqczze
Use is not syntax where appropriate and remove unnecessary parenthe…
a65aee1
…ses (PowerShell#13323)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan adityapatwardhan is a code owner
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
+1 more reviewer
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) vexx32 approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@daxian-dbw Dongbo Wang (daxian-dbw)

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.6

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@xtqqczze @rjmholt @daxian-dbw @PoshChan @baosj0 @vexx32 @TravisEz13
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.