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

[release/6.0] Update IntelliSense package #60488

Merged
merged 1 commit into from
Oct 18, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 15, 2021

This PR was merged accidentally then backed out. Please see the updated PR: #60621

@ghost
Copy link

ghost commented Oct 15, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #60487 to release/6.0

/cc @carlossanlop

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@carlossanlop carlossanlop added the Servicing-consider Issue for next servicing release review label Oct 15, 2021
@carlossanlop carlossanlop self-assigned this Oct 15, 2021
@@ -165,7 +165,7 @@
<FsCheckVersion>2.14.3</FsCheckVersion>
<SdkVersionForWorkloadTesting>6.0.100-rtm.21480.21</SdkVersionForWorkloadTesting>
<!-- Docs -->
<MicrosoftPrivateIntellisenseVersion>6.0.0-preview-20210916.1</MicrosoftPrivateIntellisenseVersion>
<MicrosoftPrivateIntellisenseVersion>6.0.0-preview-20211015.1</MicrosoftPrivateIntellisenseVersion>
Copy link
Member

Choose a reason for hiding this comment

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

It might be interesting to just do a diff test of this to make sure nothing stands out (like dropped files or something). I don't think we have a lot of validation for that. It could be a cheap test to do to reduce risk: use Beyond Compare or something to diff the two nuget package directories.

Copy link
Member

Choose a reason for hiding this comment

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

@ericstj @safern I created a branch in the api-catalog-infra repo where I uploaded the contents of the contents of the last nuget package (RC1) as the baseline, then I submitted this PR with the contents of the RTM package, to see the diff.

The diff looks good for the modified files, but I noticed there is a new file: IntellisenseFiles/net-6.0/System.Text.Json.SourceGeneration.xml . Do you think having this xml would be problematic?

Copy link
Member

Choose a reason for hiding this comment

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

Update: I realized I decompressed the zip file incorrectly. I first had to delete everything from the baseline folder, then extract the contents of the new file in place. The WPF and WinForms xml files are all missing. I'll investigate this with the Docs team.

Copy link
Member

Choose a reason for hiding this comment

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

This PR was accidentally merged, but it was promptly reverted. I will create a new backport PR.

@carlossanlop carlossanlop added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 18, 2021
@Anipik Anipik merged commit f39a90b into release/6.0 Oct 18, 2021
@carlossanlop
Copy link
Member

It had the No Merge label because of the issue mentioned in the comments above. We first need it to get fixed, then we will generate a new package with the fix, make sure it includes the missing xmls, and then we can merge it.

Should we revert this? @Anipik

@Anipik
Copy link
Contributor

Anipik commented Oct 18, 2021

ah sorry i missed it, we should revert it.

Anipik added a commit that referenced this pull request Oct 18, 2021
Anipik added a commit that referenced this pull request Oct 18, 2021
@carlossanlop carlossanlop deleted the backport/pr-60487-to-release/6.0 branch October 19, 2021 14:03
@ghost ghost locked as resolved and limited conversation to collaborators Nov 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) Servicing-consider Issue for next servicing release review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants