-
Notifications
You must be signed in to change notification settings - Fork 60
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
fix(readme): Fixing Links #1228
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -399,7 +399,7 @@ async function processModule (projectDir, manifest, branchName, repoUrl, homePag | |
} | ||
|
||
await checkLicenseFiles(projectDir) | ||
await checkReadme(projectDir, repoUrl, branchName, rootManifest) | ||
await checkReadme(projectDir, homePage, branchName, rootManifest) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this fixes the the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is the fix that is required, please rename this variable throughout To expect one and receive the other is a recipe for complete chaos. Also, the homePage generation code has |
||
} | ||
|
||
export default new Listr([ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,41 +1,39 @@ | ||
/** | ||
* @type {Record<string, (repoOwner: string, repoName: string, defaultBranch: string) => string>} | ||
* @typedef {import('../../types').ReadmeStringGeneratorInputOptions} ReadmeStringGeneratorInputOptions | ||
* @type {Record<string, (options: Omit<ReadmeStringGeneratorInputOptions, 'repoUrl'> & { defaultBranch: string}) => string>} | ||
*/ | ||
const BADGES = { | ||
libp2p: (repoOwner, repoName, defaultBranch) => ` | ||
libp2p: ({ repoOwner, repoName, defaultBranch }) => ` | ||
[![libp2p.io](https://img.shields.io/badge/project-libp2p-yellow.svg?style=flat-square)](http://libp2p.io/) | ||
[![Discuss](https://img.shields.io/discourse/https/discuss.libp2p.io/posts.svg?style=flat-square)](https://discuss.libp2p.io) | ||
[![codecov](https://img.shields.io/codecov/c/github/${repoOwner}/${repoName}.svg?style=flat-square)](https://codecov.io/gh/${repoOwner}/${repoName}) | ||
[![CI](https://img.shields.io/github/actions/workflow/status/${repoOwner}/${repoName}/js-test-and-release.yml?branch=${defaultBranch}&style=flat-square)](https://github.com/${repoOwner}/${repoName}/actions/workflows/js-test-and-release.yml?query=branch%3A${defaultBranch}) | ||
`, | ||
ipfs: (repoOwner, repoName, defaultBranch) => ` | ||
ipfs: ({ repoOwner, repoName, defaultBranch }) => ` | ||
[![ipfs.tech](https://img.shields.io/badge/project-IPFS-blue.svg?style=flat-square)](https://ipfs.tech) | ||
[![Discuss](https://img.shields.io/discourse/https/discuss.ipfs.tech/posts.svg?style=flat-square)](https://discuss.ipfs.tech) | ||
[![codecov](https://img.shields.io/codecov/c/github/${repoOwner}/${repoName}.svg?style=flat-square)](https://codecov.io/gh/${repoOwner}/${repoName}) | ||
[![CI](https://img.shields.io/github/actions/workflow/status/${repoOwner}/${repoName}/js-test-and-release.yml?branch=${defaultBranch}&style=flat-square)](https://github.com/${repoOwner}/${repoName}/actions/workflows/js-test-and-release.yml?query=branch%3A${defaultBranch}) | ||
`, | ||
multiformats: (repoOwner, repoName, defaultBranch) => ` | ||
multiformats: ({ repoOwner, repoName, defaultBranch }) => ` | ||
[![multiformats.io](https://img.shields.io/badge/project-IPFS-blue.svg?style=flat-square)](http://multiformats.io) | ||
[![codecov](https://img.shields.io/codecov/c/github/${repoOwner}/${repoName}.svg?style=flat-square)](https://codecov.io/gh/${repoOwner}/${repoName}) | ||
[![CI](https://img.shields.io/github/actions/workflow/status/${repoOwner}/${repoName}/js-test-and-release.yml?branch=${defaultBranch}&style=flat-square)](https://github.com/${repoOwner}/${repoName}/actions/workflows/js-test-and-release.yml?query=branch%3A${defaultBranch}) | ||
`, | ||
default: (repoOwner, repoName, defaultBranch) => ` | ||
default: ({ repoOwner, repoName, defaultBranch }) => ` | ||
[![codecov](https://img.shields.io/codecov/c/github/${repoOwner}/${repoName}.svg?style=flat-square)](https://codecov.io/gh/${repoOwner}/${repoName}) | ||
[![CI](https://img.shields.io/github/actions/workflow/status/${repoOwner}/${repoName}/js-test-and-release.yml?branch=${defaultBranch}&style=flat-square)](https://github.com/${repoOwner}/${repoName}/actions/workflows/js-test-and-release.yml?query=branch%3A${defaultBranch}) | ||
` | ||
} | ||
|
||
/** | ||
* @param {*} pkg | ||
* @param {string} repoOwner | ||
* @param {string} repoName | ||
* @param {string} defaultBranch | ||
* @type {import('../../types').ReadmeStringGenerator} | ||
*/ | ||
export const HEADER = (pkg, repoOwner, repoName, defaultBranch) => { | ||
export const HEADER = ({ defaultBranch, pkg, repoOwner, repoName, repoUrl }) => { | ||
return ` | ||
# ${pkg.name} <!-- omit in toc --> | ||
# [${pkg.name}](${repoUrl}) <!-- omit in toc --> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Every module on npm has a Turning this into a link also messes up the TOC generation as it takes the I think this is supposed to solve:
from ipfs/helia#35 but I think we'd be better off adding a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for a package the I can instead modify the |
||
|
||
${(BADGES[repoOwner] ?? BADGES.default)(repoOwner, repoName, defaultBranch).trim()} | ||
${(BADGES[repoOwner] ?? BADGES.default)({ repoOwner, repoName, defaultBranch }).trim()} | ||
|
||
> ${pkg.description} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -360,6 +360,25 @@ interface RunOptions { | |
bail?: boolean | ||
} | ||
|
||
interface ReadmeStringGeneratorInputOptions { | ||
repoOwner: string | ||
repoName: string | ||
repoUrl: string | ||
} | ||
|
||
interface LicenseStringGenerator { | ||
(options: ReadmeStringGeneratorInputOptions): string | ||
} | ||
|
||
interface ReadmeStringGenerator { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. additional types. |
||
(options: ReadmeStringGeneratorInputOptions & { | ||
defaultBranch: string | ||
pkg: typeof import('../package.json') & { | ||
workspaces?: string[] | ||
} | ||
}): string | ||
} | ||
|
||
export type { | ||
PartialOptions, | ||
Options, | ||
|
@@ -374,5 +393,8 @@ export type { | |
ReleaseRcOptions, | ||
DependencyCheckOptions, | ||
ExecOptions, | ||
RunOptions | ||
RunOptions, | ||
LicenseStringGenerator, | ||
ReadmeStringGenerator, | ||
ReadmeStringGeneratorInputOptions | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that passing an object for options can make the code more flexible, but now the argument types for
HEADER
,LICENSE
,APIDOCS
andSTRUCTURE
are inconsistent, since some take an object and some take multiple strings.Please can you make them consistent - either leave it as it was with multiple string arguments, or convert them all to take option arguments.