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

Collect assets file change telemetry #6207

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

nkolev92
Copy link
Member

@nkolev92 nkolev92 commented Dec 21, 2024

Bug

Related: NuGet/Home#6987

Description

I have a theory that a very, very large majority of force restore (or effective VS rebuild) events do not change the assets file.
The idea is that we don't need to do force restore during a rebuild, thus saving CPU time.

In this change, I'll add new properties to the restore event, to help me validate that theory.

  • ForceRestore (I know the name is opposite from the property, but that ship has sailed, in vs/nuget/restoreinformation we already have this property so this just makes the values consistent).
  • UpdatedAssetsFile - indicates whether the assets file needs to be updated as a consequence of this restore
  • UpdatedMSBuildFiles - indicates whether the msbuild files need to be updated as a consequence of this restore
  • IsPackageInstallationTrigger - tells that a restore was triggered due to a PM UI or PMC package installation. In this case, we should ignore any no-op specifics.

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Dec 28, 2024
@nkolev92 nkolev92 reopened this Jan 6, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jan 6, 2025
@nkolev92 nkolev92 marked this pull request as ready for review January 6, 2025 18:15
@nkolev92 nkolev92 requested a review from a team as a code owner January 6, 2025 18:15
jeffkl
jeffkl previously approved these changes Jan 6, 2025
@jeffkl
Copy link
Contributor

jeffkl commented Jan 6, 2025

Looks like tests need to be updated as well...

@nkolev92
Copy link
Member Author

nkolev92 commented Jan 6, 2025

Yeah, I returned from vacation awnd just marked it as ready for review, I thought I had done all the changes, but turns out that's not the case. :D

@nkolev92
Copy link
Member Author

nkolev92 commented Jan 6, 2025

@jeffkl ready now, tests should be passing.

jeffkl
jeffkl previously approved these changes Jan 6, 2025
@nkolev92 nkolev92 force-pushed the dev-nkolev92-collectAssetsFileChangeTelemetry branch from aee67e2 to adf6e52 Compare January 6, 2025 21:31
@nkolev92 nkolev92 enabled auto-merge (squash) January 6, 2025 21:31
@nkolev92 nkolev92 requested a review from jeffkl January 6, 2025 21:31
@nkolev92 nkolev92 merged commit 4e3472e into dev Jan 6, 2025
20 of 23 checks passed
@nkolev92 nkolev92 deleted the dev-nkolev92-collectAssetsFileChangeTelemetry branch January 6, 2025 22:48
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.

2 participants