Skip to content

Merge main-vs-deps to main#60515

Closed
JoeRobich wants to merge 0 commit intomainfrom
merges/main-to-main-vs-deps
Closed

Merge main-vs-deps to main#60515
JoeRobich wants to merge 0 commit intomainfrom
merges/main-to-main-vs-deps

Conversation

@JoeRobich
Copy link
Copy Markdown
Member

No description provided.

@JoeRobich JoeRobich requested review from a team as code owners March 31, 2022 21:08
@JoeRobich JoeRobich requested a review from a team March 31, 2022 21:08
@JoeRobich JoeRobich requested a review from a team as a code owner March 31, 2022 21:08
@ghost ghost added the Area-Infrastructure label Mar 31, 2022
Copy link
Copy Markdown
Contributor

@allisonchou allisonchou left a comment

Choose a reason for hiding this comment

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

Has it been confirmed that f5 doesn't break in public preview builds? (I can try it out later if no one has yet)

@JoeRobich JoeRobich force-pushed the merges/main-to-main-vs-deps branch from 50fdd06 to 89b1bd9 Compare March 31, 2022 23:29
@JoeRobich
Copy link
Copy Markdown
Member Author

Currently merge doesn't build because a property was removed from the main branch. see #60509 (comment)

@JoeRobich JoeRobich requested a review from a team as a code owner April 1, 2022 17:10
@JoeRobich JoeRobich force-pushed the merges/main-to-main-vs-deps branch from 3469df4 to 23f2062 Compare April 1, 2022 22:56
@JoeRobich
Copy link
Copy Markdown
Member Author

Has it been confirmed that f5 doesn't break in public preview builds? (I can try it out later if no one has yet)

@allisonchou I did some light C# work in an f5 session and nothing was broken.

ITextViewRoleSet previewRoleSet,
IGlobalOptionService globalOptions)
{
threadingContext.ThrowIfNotOnUIThread();
Copy link
Copy Markdown
Member

@dibarbet dibarbet Apr 6, 2022

Choose a reason for hiding this comment

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

Why this change?
Currently all preview window integration tests are failing because they are hitting this assert (this is not being called from the UI thread). I removed the assert as we didn't appear to need it for this ctor.

Why are we hitting this only in this PR?

  1. Cyrus merged a change in main-vs-deps which changed the behavior here to throw if not on the UI thread when previously it only asserted a UI thread existed (per Sam's comment here)
  2. This passed all PR checks originally because it was being called on the UI thread in main-vs-deps.
  3. However, a recent change in main modified this to no longer be called on the UI thread.
  4. So now when we merge, the assert (from main-vs-deps) throws because its no longer on the UI thread (from main). Fun!

@dotnet-bot dotnet-bot closed this Apr 6, 2022
@dotnet-bot dotnet-bot force-pushed the merges/main-to-main-vs-deps branch from 1160a11 to c8ebc86 Compare April 6, 2022 06:02
@sharwell sharwell deleted the merges/main-to-main-vs-deps branch April 6, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants