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

Remove missing project references in static graph-based restore #5068

Merged
merged 4 commits into from
Mar 2, 2023

Conversation

jeffkl
Copy link
Contributor

@jeffkl jeffkl commented Feb 27, 2023

Bug

Fixes: NuGet/Home#12322

Regression? Last working version:

Description

Static graph-based restore will leave project references for projects that don't actually exist in the graph. Regular restore manually removes any projects that don't support restore so this update just adds a call to the existing method. In performance testing, this added very little overhead.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@jeffkl jeffkl requested a review from a team as a code owner February 27, 2023 23:50
@jeffkl jeffkl self-assigned this Feb 27, 2023
@jeffkl jeffkl changed the title Dev jeffkl ignore missing project references Remove missing project references in static graph-based restore Feb 28, 2023
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

Looks good. There's some leftover comments in the test.

nkolev92
nkolev92 previously approved these changes Feb 28, 2023
aortiz-msft
aortiz-msft previously approved these changes Feb 28, 2023
Copy link
Contributor

@aortiz-msft aortiz-msft left a comment

Choose a reason for hiding this comment

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

Some optional suggestions from me captured during the team review meeting.

nkolev92
nkolev92 previously approved these changes Feb 28, 2023
@jeffkl jeffkl force-pushed the dev-jeffkl-ignore-missing-project-references branch from f341bf2 to 969926b Compare March 1, 2023 16:10
@jeffkl jeffkl force-pushed the dev-jeffkl-ignore-missing-project-references branch from 969926b to 60dfa6d Compare March 1, 2023 22:58
@jeffkl jeffkl merged commit 0e552e0 into dev Mar 2, 2023
@jeffkl jeffkl deleted the dev-jeffkl-ignore-missing-project-references branch March 2, 2023 16:47
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.

Static graph restore failure when referencing unrestorable project
4 participants