Skip to content
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

[main] Fix strict-mode break #7564

Merged
merged 1 commit into from
Jun 26, 2021
Merged

[main] Fix strict-mode break #7564

merged 1 commit into from
Jun 26, 2021

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Jun 25, 2021

  • use Get-Member instead of assuming $null property value
  • fixes build break I caused in 13040ff

To double check:

Tested fix locally in the dotnet/aspnetcore repo

- use `Get-Member` instead of assuming `$null` property value
- fixes build break I caused in 13040ff
@dougbu
Copy link
Member Author

dougbu commented Jun 25, 2021

/fyi @rainersigwald @ryalanms

@dougbu
Copy link
Member Author

dougbu commented Jun 25, 2021

Question for all: How did 13040ff get past the CI❔ The build was green for both #7559 and #7558 even though taking the change into dotnet/wpf failed in dotnet/wpf#4750

@dougbu dougbu merged commit 5fb184c into main Jun 26, 2021
@dougbu dougbu deleted the dougbu/fix.prefer64bit/main branch June 26, 2021 00:33
@dougbu dougbu mentioned this pull request Jun 28, 2021
1 task
@dougbu
Copy link
Member Author

dougbu commented Jun 28, 2021

OIC the break wouldn't be detected unless part of the build required desktop msbuild from VS. Does this repo or dotnet/arcade-validation contain any such tests❔ If not, should one of them❔

@riarenas
Copy link
Member

We exercise the desktop msbuild path with the usage of the signcheck tool during the signing validation job: IE. https://dev.azure.com/dnceng/internal/_build/results?buildId=1209416&view=logs&j=b11b921d-8982-5bb3-754b-b114d42fd804&t=cdcedd1f-8008-523f-9da1-cc35fbfef9a3, but there are no explicit tests for this, and clearly there's a hole since this PR had to be fix things.

@dougbu
Copy link
Member Author

dougbu commented Jun 28, 2021

clearly there's a hole since this PR had to be fix things

Agreed. That build step used an xcopy msbuild and might not have exercised the codez I changed.

lukas-lansky pushed a commit to lukas-lansky/arcade that referenced this pull request Jul 9, 2021
- use `Get-Member` instead of assuming `$null` property value
- fixes build break I caused in 13040ff
riarenas pushed a commit that referenced this pull request Jul 9, 2021
- use `Get-Member` instead of assuming `$null` property value
- fixes build break I caused in 13040ff

Co-authored-by: Doug Bunting <6431421+dougbu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants