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

add System.Reflection.Metadata 6.0.1 #506

Merged

Conversation

oleksandr-didyk
Copy link
Contributor

Done as part of dotnet/runtime#81468

runtime is manually enforcing the patched 6.0.1 version through Versions.props as part of dotnet/runtime#73871

@MichaelSimons
Copy link
Member

General comments:

  1. When adding new packages, I typically revert any edits to existing packages to reduce the burden on folks reviewing the changes. It is best to do this clean up either in a separate PR or at a minimum a separate commit.
  2. It's a good idea to validate changes locally prior to opening a PR in this repo. The build is fairly quick. Your changes fail to build because of [SBRP] generate.sh doesn't handle "<icon>" in reference package nuspecs source-build#1957.
  3. I am concerned this package is going to get accidentally cleaned up next time we do a scan for unreferenced SBRPs. The scan happens in the source-build context. What is the change being made to runtime that will get it to use the SBRP in the source-build context versus picking up latest. This is what is required for this package to not get cleaned up.

@MichaelSimons
Copy link
Member

I forgot one more thing:

  1. Any new projects need to be added as a DependencyPackageProjects here

@oleksandr-didyk
Copy link
Contributor Author

General comments:

  1. When adding new packages, I typically revert any edits to existing packages to reduce the burden on folks reviewing the changes. It is best to do this clean up either in a separate PR or at a minimum a separate commit.
  2. It's a good idea to validate changes locally prior to opening a PR in this repo. The build is fairly quick. Your changes fail to build because of [SBRP] generate.sh doesn't handle "" in reference package nuspecs source-build#1957.
  3. I am concerned this package is going to get accidentally cleaned up next time we do a scan for unreferenced SBRPs. The scan happens in the source-build context. What is the change being made to runtime that will get it to use the SBRP in the source-build context versus picking up latest. This is what is required for this package to not get cleaned up.
  1. Good point, thank you. Removed the unrelated changes.
  2. That's on me, sorry. Removed the icon tag and re-ran the build locally, should be A-OK now
  3. The package version added is already in use in runtime, meaning that once dependency on SBRP is added to runtime, its source-build should pick this version up. The SBRP dependency should be added as part of Enable source-build pre-built detection runtime#81468 (If I understood correctly your point here)
  1. Any new projects need to be added as a DependencyPackageProjects here
  1. Thanks for the pointer, added

@oleksandr-didyk oleksandr-didyk mentioned this pull request Feb 2, 2023
@oleksandr-didyk oleksandr-didyk self-assigned this Feb 2, 2023
@MichaelSimons
Copy link
Member

Regarding #4, once you complete the changes to enable prebuilt detection for runtime, it would be good to validate in a full source-build. After validating that, it would be good to enable the per-repo pvp flow for runtime to validate it works and then this SBRP will get referenced.

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