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

Val/revert66430 17.6 #67849

Merged

Conversation

chsienki
Copy link
Contributor

@chsienki chsienki commented Apr 17, 2023

Port of #67808 (a revert PR) to 17.6 for servicing
Corresponding DevDiv issue: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1733125

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 17, 2023
@jaredpar jaredpar marked this pull request as ready for review April 27, 2023 18:00
@jaredpar jaredpar requested a review from a team as a code owner April 27, 2023 18:00
NuGet.config Outdated
@@ -12,7 +12,9 @@
<add key="dotnet-public" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json" />
<add key="nuget-build" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/nuget-build/nuget/v3/index.json" />
<add key="vssdk" value="https://pkgs.dev.azure.com/azure-public/vside/_packaging/vssdk/nuget/v3/index.json" />
<add key="vssdk-archived" value="https://pkgs.dev.azure.com/azure-public/vside/_packaging/vssdk-archived/nuget/v3/index.json" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes in this file look unrelated to the revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VS has changed the way they handle NuGet feeds since we last inserted this branch. This is a cherry pick from main that is required to build this branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a cherry pick from main that is required to build this branch.

The change shouldn't be part of this PR, unless it is specific to the change we are making. Which it sounds like it is not related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I can break it out into two PRs, but they'll be inserted in the same VS insertion either way. We can't insert without the feed change.

@AlekseyTs
Copy link
Contributor

Done with review pass (commit 2). It looks like CI jobs are failing, BTW.

@chsienki
Copy link
Contributor Author

chsienki commented May 3, 2023

Updated to only include the single commit

@jcouv jcouv requested a review from AlekseyTs May 3, 2023 23:07
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 1)

@jcouv jcouv removed the untriaged Issues and PRs which have not yet been triaged by a lead label May 3, 2023
@jcouv jcouv added this to the 17.6 milestone May 3, 2023
@jcouv jcouv merged commit d2f3bcc into dotnet:release/dev17.6-vs-deps May 4, 2023
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