Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Use is not syntax and remove unnecessary parentheses#13323

Merged
daxian-dbw merged 6 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:isnotxtqqczze/PowerShell-PowerShell:isnotCopy head branch name to clipboard
Jul 31, 2020
Merged

Use is not syntax and remove unnecessary parentheses#13323
daxian-dbw merged 6 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:isnotxtqqczze/PowerShell-PowerShell:isnotCopy head branch name to clipboard

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

@xtqqczze
Use is not syntax
bf0f1ea 
-replace '\(!\(([^)]+) is ([^)]+)\)\)', '($1 is not $2)'
@ghost ghost assigned daxian-dbw Jul 30, 2020
@xtqqczze xtqqczze marked this pull request as ready for review July 30, 2020 23:41
@xtqqczze xtqqczze mentioned this pull request Jul 30, 2020
Closed
14 tasks
@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

@daxian-dbw Can you restart CI?

@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

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

@rjmholt

rjmholt commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

/azp help

@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.

@rjmholt

rjmholt commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

/azp list

@azure-pipelines

azure-pipelines Bot commented Jul 31, 2020

Copy link
Copy Markdown
CI/CD Pipelines for this repository:

@rjmholt

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

@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

/azp run

@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

vexx32
vexx32 suggested changes Jul 31, 2020
View reviewed changes

@vexx32 vexx32 left a comment

Copy link
Copy Markdown
Collaborator

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. 🙂

Comment thread src/System.Management.Automation/engine/TypeTable.cs Outdated
Comment thread src/System.Management.Automation/engine/hostifaces/ConnectionFactory.cs Outdated
Comment thread src/System.Management.Automation/engine/hostifaces/ConnectionFactory.cs Outdated
Comment thread src/System.Management.Automation/engine/parser/Compiler.cs Outdated
@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
daxian-dbw requested changes Jul 31, 2020
View reviewed changes
Comment thread ...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionAnalysis.cs Outdated
Comment thread src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
@xtqqczze @vexx32
Apply suggestions from code review
bd1f4c4 
Co-authored-by: Joel Sallow (/u/ta11ow) <32407840+vexx32@users.noreply.github.com>
@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 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
vexx32
vexx32 approved these changes Jul 31, 2020
View reviewed changes

@vexx32 vexx32 left a comment

Copy link
Copy Markdown
Collaborator

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! 💖

@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.

daxian-dbw
daxian-dbw approved these changes Jul 31, 2020
View reviewed changes

@daxian-dbw daxian-dbw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!

@daxian-dbw

daxian-dbw commented Jul 31, 2020

Copy link
Copy Markdown
Member

@PoshChan Please remind me in 1 hour

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

PoshChan commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

@daxian-dbw, this is the reminder you requested 1 hour ago

@daxian-dbw daxian-dbw merged commit 4597b41 into PowerShell:master Jul 31, 2020
@daxian-dbw 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 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
Merged
8 tasks
@baosj0

baosj0 commented Aug 2, 2020

Copy link
Copy Markdown

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

@xtqqczze

xtqqczze commented Aug 2, 2020 •
edited
Loading

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).

This was referenced Aug 2, 2020
Merged
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

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

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:

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

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan adityapatwardhan is a code owner

@anmenaga anmenaga Awaiting requested review from anmenaga

+1 more reviewer

@vexx32 vexx32 vexx32 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@daxian-dbw 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.

7 participants

@xtqqczze @rjmholt @daxian-dbw @PoshChan @baosj0 @vexx32 @TravisEz13
Morty Proxy This is a proxified and sanitized view of the page, visit original site.