Skip to content

Refactor "PDB distributions" section of README#1271

Merged
tmat merged 6 commits intodotnet:mainfrom
MattKotsenas:bugfix/readme-embedded
Feb 13, 2025
Merged

Refactor "PDB distributions" section of README#1271
tmat merged 6 commits intodotnet:mainfrom
MattKotsenas:bugfix/readme-embedded

Conversation

@MattKotsenas
Copy link
Member

@MattKotsenas MattKotsenas commented Aug 28, 2024 •
edited
Loading

Refactor the "PDB distributions" section of the README so that:

  • Add a small pros / cons table explaining the benefits and limitations of: .snupkgs, Adding to the main package, and assembly embedding
  • There's less bouncing back and forth between text (specifically shrinking the distance between introducing a .snupkg and talking about the limitations)
  • Add explanation of embedding and benefits / limitations
  • Update "main package" alternative to make it clear that it requires consumers to set CopyDebugSymbolFilesFromPackages
  • Use GitHub markdown tags for asides or secondary information

Fixes #255

@MattKotsenas
Copy link
Member Author

MattKotsenas commented Aug 28, 2024 •
edited
Loading

I originally wrote this change as part of #1270, but separated it into its own PR since this one is a bit more nuanced and contains some opinions instead of just facts.

More specifically, the only downside I see for advocating for embedded pdbs over separate files included in the main package is it's easier to strip separate files for people that really care about size. I'm not convinced that requiring every consumer to opt-in via CopyDebugSymbolFilesFromPackages is a good tradeoff though, and they should instead either publish on a platform that supports snupkg or use another solution like symbol servers.

@MattKotsenas
Copy link
Member Author

MattKotsenas commented Aug 28, 2024

@tmat, would you mind taking a look at this when you have a second? If you have any questions or concerns, please let me know. Thanks!

tmat
tmat reviewed Dec 23, 2024
View reviewed changes
@ViktorHofer ViktorHofer removed their request for review January 13, 2025 19:49
@MattKotsenas MattKotsenas requested a review from tmat January 13, 2025 19:50
@MattKotsenas
Copy link
Member Author

MattKotsenas commented Feb 10, 2025

/cc @tmat, mind making another review? If you have any questions / concerns, please let me know. Thanks!

tmat
tmat reviewed Feb 11, 2025
View reviewed changes
tmat
tmat reviewed Feb 11, 2025
View reviewed changes
tmat
tmat reviewed Feb 11, 2025
View reviewed changes
@MattKotsenas
Remove "all types" from chart for PDB distributions
9a7d1b6 
Since Windows-only PDBs are legacy for managed code, and this section is
about managed code, the "supports all types" row isn't really relvant.
Remove it.
@MattKotsenas
Copy edits for note about embedding size
96c3015
@tmat tmat merged commit 0dd14e7 into dotnet:main Feb 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@baronfel baronfel baronfel approved these changes

@tmat tmat tmat approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

guidance for embedding pdb in nupkg vs snupkg

3 participants

@MattKotsenas @tmat @baronfel