Skip to content

Conversation

@stephentoub
Copy link
Contributor

Somehow we ended up with two, one in the root, one in the src folder. This deletes the latter.

Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

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

This is the README used by the NuGet package. There is some duplication but I wouldn't expect these to be identical.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Mar 27, 2025

@jeffhandley Might make sense to include a dotnet pack step in ci.yml so that breaks to packaging are detected in PRs.

@stephentoub
Copy link
Contributor Author

There is some duplication but I wouldn't expect these to be identical.

There is a lot of duplication.

Why wouldn't they be identical? The only places they're not right now highlight the problem, which is that changes to the real README weren't replicated to the copy.

I don't want us to maintain two different copies.

@eiriktsarpalis
Copy link
Member

We can get rid of the code samples from the package README for now if that's a concern. I just don't think these files should be identical, a repo README for example might have instructions for the dev workflow or contribution guidelines that would be inappropriate for the package.

@stephentoub
Copy link
Contributor Author

stephentoub commented Mar 27, 2025

My high-level is I don't want two copies of the same content, even partially (other than a paragraph or the like). And right now, they're the same thing (or, rather, worse, they're trying to be the same thing and failing at it). I'm not sure why we'd delete the code samples from the package, either.

@eiriktsarpalis
Copy link
Member

This problem can be solved by drastically trimming the size of the package README and perhaps linking to the repo README from it (although we already point to the repo through nuspec metadata).

@eiriktsarpalis
Copy link
Member

I foresee this would be less of a problem once the API stabilizes. In the STJ README for example we include a very basic code sample that we know isn't going to change.

@stephentoub
Copy link
Contributor Author

stephentoub commented Mar 27, 2025

Right now I see no reason not to just use the same one.

If we decide we want a minimalistic, separate README for the nuget package, that's not duplicating the main one and is instead focused on specifics of nuget, we can add that and remove the ..\..\ at that time.

@eiriktsarpalis
Copy link
Member

Right now I see no reason not to just use the same one.

The one issue I can see right now is that we include a NuGet package badge at the top of the document. I do expect divergences to increase as the repo evolves though so I think establishing separation is important. I think starting with a minimalistic package README that links to this repo, the API docs, and the main MCP website is fine.

@stephentoub
Copy link
Contributor Author

The one issue I can see right now is that we include a NuGet package badge at the top of the document.

Lots of packages just use the same readme and have such badges, e.g.
https://www.nuget.org/packages/OpenAI/2.2.0-beta.4#readme-body-tab

@eiriktsarpalis eiriktsarpalis self-requested a review March 27, 2025 18:12
@eiriktsarpalis eiriktsarpalis dismissed their stale review March 27, 2025 18:12

Dismiss my review

@jeffhandley
Copy link
Collaborator

@stephentoub Thanks for catching this. It's a relic from last week's publishing effort and I didn't get it cleaned up yet. My PR for #118 tweaked the wording to make the README reusable across the repo and the package, so I'd like to close this PR and address the feedback above as part of #118.

@jeffhandley
Copy link
Collaborator

@jeffhandley Might make sense to include a dotnet pack step in ci.yml so that breaks to packaging are detected in PRs.

Good call. I am incorporating that into #118.

As for separating vs. reusing, I agree with @stephentoub's recommendation of just reusing the same file for now, while anticipating a separation at some time in the future. For the short-term, I think it's valuable to have the full README content showing up on nuget.org when folks find the package as it emphasizes this package as the official SDK.

@stephentoub
Copy link
Contributor Author

so I'd like to close this PR and address the feedback above as part of #118.

Ok

@stephentoub stephentoub deleted the deletereadme branch March 31, 2025 14:27
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.

3 participants