Skip to content

Revert the PRs that made DBNull.Value and NullString.Value treated as $null #11584

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.

Sign up for GitHub

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

Jump to bottom
Merged
adityapatwardhan merged 5 commits into from
Jan 15, 2020
Merged

Revert the PRs that made DBNull.Value and NullString.Value treated as $null #11584

adityapatwardhan merged 5 commits into from
Jan 15, 2020

Conversation

@daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Jan 14, 2020 •
edited
Loading

PR Summary

Revert the PRs that made DBNull.Value and NullString.Value treated as $null:

Also update code for Coalescing and Null-conditional operators to use 'IsNull' instead.

PR Checklist

adityapatwardhan
adityapatwardhan approved these changes Jan 14, 2020
View reviewed changes
Copy link
Member

@adityapatwardhan adityapatwardhan left a comment

Choose a reason for hiding this comment

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

Approved with nitpicks

@daxian-dbw
Copy link
Member Author

daxian-dbw commented Jan 14, 2020

@adityapatwardhan You just need to review the 3rd commit. The previous 2 are from git revert those 2 PRs.

@daxian-dbw
Copy link
Member Author

daxian-dbw commented Jan 14, 2020

@adityapatwardhan @TravisEz13 Also, the windows build failed at the packaging phase with a "calling method on null-value" error:

You cannot call a method on a null-valued expression.
at Invoke-CIFinish, D:\a\1\s\tools\ci.psm1: line 452
at <ScriptBlock>, D:\a\_temp\00413339-7785-4f63-ab8a-bab4a33c6cbe.ps1: line 4
at <ScriptBlock>, <No file>: line 1
You cannot call a method on a null-valued expression.
At D:\a\1\s\tools\ci.psm1:452 char:9
+         $previewLabel = $previewVersion[1].replace('.','')
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (:) [], RuntimeException
    + FullyQualifiedErrorId : InvokeMethodOnNull

See https://dev.azure.com/powershell/PowerShell/_build/results?buildId=43557&view=logs&j=1057fe42-c88b-5deb-88ad-59245c714665&t=1888043b-254d-5761-80ec-47d0d8adc59a

@SteveL-MSFT SteveL-MSFT added this to the GA-consider milestone Jan 15, 2020
@adityapatwardhan adityapatwardhan merged commit d99b325 into PowerShell:release/v7.0.0-rc.2 Jan 15, 2020
@daxian-dbw daxian-dbw deleted the revert branch January 15, 2020 00:47
@adityapatwardhan
Copy link
Member

adityapatwardhan commented Jan 15, 2020

@SteveL-MSFT The PR is targetting RC.2 branch so it is auto approved for RC.

Merged this even though there were failures in Windows packaging as we have differences in content in metadata.json in RC.2 branch. It was fixed to unblock CI with PR #11372
We do not need to take this change to the RC.2 branch.

@daxian-dbw daxian-dbw added CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log CL-Engine Indicates that a PR should be marked as an engine change in the Change Log and removed CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log labels Jan 15, 2020
@ghost
Copy link

ghost commented Jan 16, 2020

🎉v7.0.0-rc.2 has been released which incorporates this pull request.:tada:

Handy links:

@ili101
Copy link

ili101 commented Jan 17, 2020

Why was this reverted?
Is this final or are you planing to change it again in the future?
BTW probably should have been listed under the "Breaking Changes" category in the v7.0.0-rc.2 Release note.

@vexx32
Copy link
Collaborator

vexx32 commented Jan 17, 2020 •
edited
Loading

@ili101 the change was never included in a stable release, so reverting it is not a breaking change. Please see this issue for discussion -- depending on what is decided by the PS team and community, the change may be reintroduced, likely with some alterations if it is, in a later release, likely at least 7.1 🙂

daxian-dbw added a commit to daxian-dbw/PowerShell that referenced this pull request Jan 21, 2020
@daxian-dbw
Revert the PRs that made DBNull.Value and NullString.Value treate…
b90e530 
…d as `$null` (PowerShell#11584)
@daxian-dbw daxian-dbw mentioned this pull request Jan 21, 2020
Merged
14 tasks
@TravisEz13 TravisEz13 mentioned this pull request Oct 12, 2022
Merged
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga

@TravisEz13 TravisEz13 Awaiting requested review from TravisEz13

+1 more reviewer

@PoshChan PoshChan PoshChan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan adityapatwardhan

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Milestone

7.0.0-rc.2

Development

Successfully merging this pull request may close these issues.

6 participants

@daxian-dbw @adityapatwardhan @ili101 @vexx32 @PoshChan @SteveL-MSFT