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

Pass through the cache context to the vulnerability resource #5361

Merged
merged 4 commits into from
Aug 19, 2023

Conversation

nkolev92
Copy link
Member

@nkolev92 nkolev92 commented Aug 15, 2023

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/2432

Regression? Last working version:

Description

The VulnerabilityInformationProvider creates redundant SourceCacheContext. We should reuse the same SourceCacheContext throughout the whole operation.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception - Tests already exist for the enum changes.
    • OR
    • N/A
  • Documentation

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

@nkolev92 nkolev92 requested a review from a team as a code owner August 15, 2023 00:44
@nkolev92 nkolev92 force-pushed the dev-nkolev92-vulnerabilityAPICleanup branch from 8ec91f3 to 6bcba8f Compare August 15, 2023 00:46
martinrrm
martinrrm previously approved these changes Aug 15, 2023
Copy link
Contributor

@jeffkl jeffkl 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 otherwise but let's bring back the Enum.ToString() optimization

@nkolev92 nkolev92 dismissed stale reviews from martinrrm and donnie-msft via be591a4 August 17, 2023 23:57
@nkolev92 nkolev92 force-pushed the dev-nkolev92-vulnerabilityAPICleanup branch from 6bcba8f to be591a4 Compare August 17, 2023 23:57
@nkolev92 nkolev92 changed the title Remove redundant methods, pass through the cache context to the vulnerability resource Pass through the cache context to the vulnerability resource Aug 17, 2023
@nkolev92 nkolev92 requested a review from aortiz-msft August 18, 2023 00:01
@nkolev92
Copy link
Member Author

@jeffkl @aortiz-msft Addressed all feedback.

jeffkl
jeffkl previously approved these changes Aug 18, 2023
zivkan
zivkan previously approved these changes Aug 18, 2023
aortiz-msft
aortiz-msft previously approved these changes Aug 18, 2023
@nkolev92 nkolev92 dismissed stale reviews from aortiz-msft, zivkan, and jeffkl via 2c08b40 August 18, 2023 20:17
@nkolev92 nkolev92 requested review from zivkan and aortiz-msft August 18, 2023 20:18
@nkolev92 nkolev92 requested a review from jeffkl August 18, 2023 20:18
@nkolev92 nkolev92 enabled auto-merge (squash) August 18, 2023 20:18
@nkolev92 nkolev92 merged commit 9ea2def into dev Aug 19, 2023
@nkolev92 nkolev92 deleted the dev-nkolev92-vulnerabilityAPICleanup branch August 19, 2023 11:51
zivkan added a commit that referenced this pull request Aug 30, 2023
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.

6 participants