Split library and package Readme by MSDN-WhiteKnight · Pull Request #78888 · dotnet/runtime

MSDN-WhiteKnight · 2022-11-27T17:38:23Z

I've initially submitted library and package Readmes as single file for Microsoft.Extension.Configuration libraries, but maintainers raised issue that this exposes unneeded information on NuGet gallery: #59630 (comment)

So the new convention is:

README.md - Library Readme. Contains documentation for contributors, and very little info for users.
PACKAGE.md - Package Readme. Contains documentation for users (this one is published on NuGet gallery).

/cc @jeffhandley @ViktorHofer

Related to: #59630, #77733

ghost · 2022-11-27T17:38:35Z

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

I've initially submitted library and package Readmes as single file for Microsoft.Extension.Configuration libraries, but maintainers raised issue that this exposes unneeded information on NuGet gallery: #59630 (comment)

So the new convention is:

README.md - Library Readme. Contains documentation for contributors, and very little info for users.
PACKAGE.md - Package Readme. Contains documentation for users (this one is published on NuGet gallery).

/cc @jeffhandley @ViktorHofer

Related to: #59630, #77733

Author:	MSDN-WhiteKnight
Assignees:	-
Labels:	`area-Extensions-Configuration`, `community-contribution`
Milestone:	-

ghost · 2022-11-27T21:02:29Z

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

I've initially submitted library and package Readmes as single file for Microsoft.Extension.Configuration libraries, but maintainers raised issue that this exposes unneeded information on NuGet gallery: #59630 (comment)

So the new convention is:

README.md - Library Readme. Contains documentation for contributors, and very little info for users.
PACKAGE.md - Package Readme. Contains documentation for users (this one is published on NuGet gallery).

/cc @jeffhandley @ViktorHofer

Related to: #59630, #77733

Author:	MSDN-WhiteKnight
Assignees:	-
Labels:	`area-Meta`, `community-contribution`
Milestone:	-

src/libraries/Microsoft.Extensions.Configuration.Abstractions/README.md

ghost · 2022-11-27T22:48:38Z

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

I've initially submitted library and package Readmes as single file for Microsoft.Extension.Configuration libraries, but maintainers raised issue that this exposes unneeded information on NuGet gallery: #59630 (comment)

So the new convention is:

README.md - Library Readme. Contains documentation for contributors, and very little info for users.
PACKAGE.md - Package Readme. Contains documentation for users (this one is published on NuGet gallery).

/cc @jeffhandley @ViktorHofer

Related to: #59630, #77733

Author:	MSDN-WhiteKnight
Assignees:	-
Labels:	`area-Extensions-Configuration`, `community-contribution`
Milestone:	-

ghost · 2022-11-28T09:32:15Z

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

I've initially submitted library and package Readmes as single file for Microsoft.Extension.Configuration libraries, but maintainers raised issue that this exposes unneeded information on NuGet gallery: #59630 (comment)

So the new convention is:

README.md - Library Readme. Contains documentation for contributors, and very little info for users.
PACKAGE.md - Package Readme. Contains documentation for users (this one is published on NuGet gallery).

/cc @jeffhandley @ViktorHofer

Related to: #59630, #77733

Author:	MSDN-WhiteKnight
Assignees:	-
Labels:	`area-Meta`, `community-contribution`
Milestone:	8.0.0

ViktorHofer · 2022-11-28T09:38:43Z

@MSDN-WhiteKnight I hope it's OK that I pushed to your branch. I made the following refinements (three commits):

Automatically include the package readme file in the package if it exists. That reduces duplicity in every project file.
Consistently rename all packaging README.md files to PACKAGE.md.
Move all PACKAGE.md files into the "src" sub directory as the other subdirectories (ref, tests, ...) are non-shipping. Moving the PACKAGE.md file into the src directory makes it clear that the information applies to the source assembly and its package only.

jkotas · 2022-11-28T16:59:19Z

PACKAGE.md - Package Readme. Contains documentation for users (this one is published on NuGet gallery).

Does NuGet gallery look for PACKAGE.md? Or do we need to rename it to README.md as part of packing?

MSDN-WhiteKnight · 2022-11-28T17:25:15Z

PACKAGE.md - Package Readme. Contains documentation for users (this one is published on NuGet gallery).

Does NuGet gallery look for PACKAGE.md? Or do we need to rename it to README.md as part of packing?

NuGet uses file specified by PackageReadmeFile property. This property is now globally set in packaging.targets to PACKAGE.md so yes, it will be recognized.

eerhardt · 2022-11-28T17:27:16Z

eng/packaging.targets


+  <!-- Add a package README file from. -->
+  <ItemGroup Condition="'$(PackageReadmeFile)' != ''">
+    <None Include="$(PackageReadmeFile)" Pack="true" PackagePath="\" />


Suggested change

<None Include="$(PackageReadmeFile)" Pack="true" PackagePath="\" />

<None Include="$(PackageReadmeFile)" Pack="true" PackagePath="README.md" />

You want the file to be named README.md, right?

We don't have to specify name README.md, NuGet will recognize any name set by PackageReadmeFile property. Also PackagePath seems to be a path without name, it is used like that in docs: https://learn.microsoft.com/en-us/nuget/reference/msbuild-targets#packagereadmefile

eerhardt · 2022-11-28T17:28:37Z

eng/packaging.targets

    <!-- Don't include target platform specific dependencies, since we use the target platform to represent RIDs instead -->
    <SuppressDependenciesWhenPacking Condition="'$(TargetPlatformIdentifier)' != ''">true</SuppressDependenciesWhenPacking>
    <PackageDesignerMarkerFile>$(MSBuildThisFileDirectory)useSharedDesignerContext.txt</PackageDesignerMarkerFile>
+    <PackageReadmeFile Condition="'$(PackageReadmeFile)' == '' and Exists('PACKAGE.md')">PACKAGE.md</PackageReadmeFile>


Maybe the file name could be PACKAGE_README.md? PACKAGE.md doesn't really describe exactly what the file is.

When it comes to NuGet Gallery, the PACKAGE.md file is automatically respected and displayed as the package's nuspec encodes it. Personally, I'm fine with the PACKAGE.md name and I don't think it's common that a dev crack opens a nuget package in order to understand how to use it.

It's more about readability/understandability of the files in the repo.

Other options include:

PackageReadme.md

The file under a docs folder, and still name it README.md. e.g. src/docs/README.md

Since the files are split already - one is in the root of the library, and one is under src - they could both be named README.md.

The PACKAGE.md choice was based on this suggestion: #59630 (comment) I don't think this name needs to be long as there's not a lot of .md files in folder to disambiguate between them. So personally i'd prefer either README.md or PACKAGE.md.

Being able to distinguish between a package documentation file and a repo specific contribution guidance seems valuable to me. We unintentionally started shipping the contribution guidance files to customers and this PR fixes that: #59630 (comment).

eerhardt · 2022-11-28T17:29:30Z

src/libraries/System.Text.Json/src/System.Text.Json.csproj

-    <IsPackable>true</IsPackable>
    <!-- This library has been annotated to be AOT safe -->
    <EnableAOTAnalyzer>true</EnableAOTAnalyzer>
+    <IsPackable>true</IsPackable>


Why is this moved/changed? Seems unnecessary.

It was moved to group packaging properties together.

ViktorHofer · 2022-12-07T15:44:10Z

@MSDN-WhiteKnight can you please update https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/libraries-packaging.md and add a section that describes the new conventions (including mentioning/explaining that the file is automatically included in the package if conventions are followed)?

MSDN-WhiteKnight · 2022-12-08T03:51:59Z

can you please update https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/libraries-packaging.md and add a section that describes the new conventions (including mentioning/explaining that the file is automatically included in the package if conventions are followed)?

I've added a section about Package readme, and also edited PackageDescription guidelines to remove the list of entry-point types (it is now obsolete as including it in readme does it better). Should i also explain what is library readme and how it's different from package readme? In libraries-packaging.md, or in some other higher-level document?

docs/coding-guidelines/libraries-packaging.md

ViktorHofer · 2022-12-12T07:32:18Z

I've added a section about Package readme, and also edited PackageDescription guidelines to remove the list of entry-point types (it is now obsolete as including it in readme does it better). Should i also explain what is library readme and how it's different from package readme? In libraries-packaging.md, or in some other higher-level document?

Thanks a lot. Regarding libraries README files, those are already documented in the src/libraries/README.md file: https://github.com/dotnet/runtime/blob/main/src/libraries/README.md

MSDN-WhiteKnight added 2 commits November 27, 2022 22:25

Split library and package Readme

a9d577f

Fix punctuation

944daa0

ghost added community-contribution Indicates that the PR has been added by a community member area-Extensions-Configuration labels Nov 27, 2022

Fix lint issues

170adeb

teo-tsirpanis added area-Meta and removed area-Extensions-Configuration labels Nov 27, 2022

tarekgh reviewed Nov 27, 2022

View reviewed changes

src/libraries/Microsoft.Extensions.Configuration.Abstractions/README.md Show resolved Hide resolved

tarekgh added area-Extensions-Configuration and removed area-Meta labels Nov 27, 2022

tarekgh added this to the 8.0.0 milestone Nov 27, 2022

ViktorHofer added 3 commits November 28, 2022 10:21

Automatically include package readme file if existent

fc32b96

Rename README.md to PACKAGE.md

1f95da5

Move PACKAGE.md into source directory

1b0a89e

ViktorHofer added area-Meta and removed area-Extensions-Configuration labels Nov 28, 2022

build-analysis bot mentioned this pull request Nov 28, 2022

SyndicationFeed_Write_RSS_Atom test failing with "IOException : The file '/tmp/' already exists." #78454

Closed

eerhardt reviewed Nov 28, 2022

View reviewed changes

Reinstate examples in README.md

ea477be

tarekgh approved these changes Nov 28, 2022

View reviewed changes

This was referenced Nov 28, 2022

Tracking issue for CI build timeouts #76454

Closed

install-native-dependencies.sh failing in CI dotnet/arcade#11740

Closed

MSDN-WhiteKnight and others added 2 commits November 29, 2022 23:14

Merge remote-tracking branch 'upstream/main' into package-readme

053003e

Merge branch 'main' into package-readme

f091280

build-analysis bot mentioned this pull request Dec 6, 2022

iOS & tvOS legs are failing to AOT System.Net.Http.Json #79279

Closed

Update contributing guidelines

368609c

ViktorHofer reviewed Dec 12, 2022

View reviewed changes

docs/coding-guidelines/libraries-packaging.md Outdated Show resolved Hide resolved

ViktorHofer approved these changes Dec 12, 2022

View reviewed changes

Update docs/coding-guidelines/libraries-packaging.md

af34dcf

ViktorHofer merged commit 479d013 into dotnet:main Dec 12, 2022

ViktorHofer mentioned this pull request Dec 12, 2022

Embedded markdown readme for NuGet packages #59630

Closed

7 tasks

MSDN-WhiteKnight deleted the package-readme branch December 12, 2022 11:14

This was referenced Dec 12, 2022

Precondition failure: File has not had execution verified #79439

Closed

Test failure JIT\\opt\\Tailcall\\TailcallVerifyWithPrefix\\TailcallVerifyWithPrefix.cmd #79517

Closed

ghost locked as resolved and limited conversation to collaborators Jan 11, 2023

	<None Include="$(PackageReadmeFile)" Pack="true" PackagePath="\" />
	<None Include="$(PackageReadmeFile)" Pack="true" PackagePath="README.md" />

Conversation

MSDN-WhiteKnight commented Nov 27, 2022

Uh oh!

ghost commented Nov 27, 2022

Uh oh!

ghost commented Nov 27, 2022

Uh oh!

Uh oh!

ghost commented Nov 27, 2022

Uh oh!

ghost commented Nov 28, 2022

Uh oh!

ViktorHofer commented Nov 28, 2022

Uh oh!

jkotas commented Nov 28, 2022

Uh oh!

MSDN-WhiteKnight commented Nov 28, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MSDN-WhiteKnight Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ViktorHofer Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ViktorHofer commented Dec 7, 2022

Uh oh!

MSDN-WhiteKnight commented Dec 8, 2022

Uh oh!

Uh oh!

ViktorHofer commented Dec 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

MSDN-WhiteKnight Nov 28, 2022 •

edited

Loading

ViktorHofer Nov 28, 2022 •

edited

Loading