From 73d7bf174d34c5d51b87f937daf52b94c9e803c4 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 11:20:37 -0700 Subject: [PATCH 01/11] feat: add release/v branches to all branch CI --- .github/workflows/ci-test-workspace.yml | 1 + .github/workflows/ci.yml | 1 + .github/workflows/codeql-analysis.yml | 2 ++ lib/content/index.js | 2 +- lib/content/release.yml | 1 - tap-snapshots/test/apply/source-snapshots.js.test.cjs | 10 ++++++++++ tap-snapshots/test/check/diff-snapshots.js.test.cjs | 2 +- 7 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci-test-workspace.yml b/.github/workflows/ci-test-workspace.yml index 797092e2..5e88d011 100644 --- a/.github/workflows/ci-test-workspace.yml +++ b/.github/workflows/ci-test-workspace.yml @@ -11,6 +11,7 @@ on: branches: - main - latest + - release/v* paths: - workspace/test-workspace/** schedule: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fede7162..00c8a131 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,6 +11,7 @@ on: branches: - main - latest + - release/v* paths-ignore: - workspace/test-workspace/** schedule: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 66b9498a..21244879 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -7,10 +7,12 @@ on: branches: - main - latest + - release/v* pull_request: branches: - main - latest + - release/v* schedule: # "At 10:00 UTC (03:00 PT) on Monday" https://crontab.guru/#0_10_*_*_1 - cron: "0 10 * * 1" diff --git a/lib/content/index.js b/lib/content/index.js index cb572e09..8b7c1d5c 100644 --- a/lib/content/index.js +++ b/lib/content/index.js @@ -131,7 +131,7 @@ module.exports = { workspaceModule, windowsCI: true, macCI: true, - branches: ['main', 'latest'], + branches: ['main', 'latest', 'release/v*'], defaultBranch: 'main', distPaths: [ 'bin/', diff --git a/lib/content/release.yml b/lib/content/release.yml index 39693f1c..7a8ea050 100644 --- a/lib/content/release.yml +++ b/lib/content/release.yml @@ -11,7 +11,6 @@ on: {{#each branches}} - {{ . }} {{/each}} - - release/v* permissions: contents: write diff --git a/tap-snapshots/test/apply/source-snapshots.js.test.cjs b/tap-snapshots/test/apply/source-snapshots.js.test.cjs index 4a101dfa..bf8ea121 100644 --- a/tap-snapshots/test/apply/source-snapshots.js.test.cjs +++ b/tap-snapshots/test/apply/source-snapshots.js.test.cjs @@ -466,6 +466,7 @@ on: branches: - main - latest + - release/v* schedule: # "At 09:00 UTC (02:00 PT) on Monday" https://crontab.guru/#0_9_*_*_1 - cron: "0 9 * * 1" @@ -574,10 +575,12 @@ on: branches: - main - latest + - release/v* pull_request: branches: - main - latest + - release/v* schedule: # "At 10:00 UTC (03:00 PT) on Monday" https://crontab.guru/#0_10_*_*_1 - cron: "0 10 * * 1" @@ -1685,6 +1688,7 @@ on: branches: - main - latest + - release/v* paths: - workspaces/a/** schedule: @@ -1799,6 +1803,7 @@ on: branches: - main - latest + - release/v* paths: - workspaces/b/** schedule: @@ -2133,6 +2138,7 @@ on: branches: - main - latest + - release/v* paths-ignore: - workspaces/a/** - workspaces/b/** @@ -2244,10 +2250,12 @@ on: branches: - main - latest + - release/v* pull_request: branches: - main - latest + - release/v* schedule: # "At 10:00 UTC (03:00 PT) on Monday" https://crontab.guru/#0_10_*_*_1 - cron: "0 10 * * 1" @@ -3355,6 +3363,7 @@ on: branches: - main - latest + - release/v* paths: - workspaces/a/** schedule: @@ -3469,6 +3478,7 @@ on: branches: - main - latest + - release/v* paths: - workspaces/b/** schedule: diff --git a/tap-snapshots/test/check/diff-snapshots.js.test.cjs b/tap-snapshots/test/check/diff-snapshots.js.test.cjs index 70d415dc..f1fb272c 100644 --- a/tap-snapshots/test/check/diff-snapshots.js.test.cjs +++ b/tap-snapshots/test/check/diff-snapshots.js.test.cjs @@ -173,7 +173,7 @@ The repo file ci.yml needs to be updated: .github/workflows/ci.yml ======================================== - @@ -83,5 +83,25 @@ + @@ -84,5 +84,25 @@ node-version: \${{ matrix.node-version }} - name: Update Windows npm # node 12 and 14 ship with npm@6, which is known to fail when updating itself in windows From 9606606f0163d56b785620881d5b01985f9218e3 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 11:20:58 -0700 Subject: [PATCH 02/11] feat: add config option to not update npm --- lib/config.js | 2 +- lib/content/index.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/config.js b/lib/config.js index f2da45d9..0c6333c3 100644 --- a/lib/config.js +++ b/lib/config.js @@ -195,7 +195,7 @@ const getFullConfig = async ({ isLatest, // whether to install and update npm in ci // only do this if we aren't using a custom path to bin - updateNpm: !npmPath.isLocal, + updateNpm: !npmPath.isLocal && pkgConfig.updateNpm, rootNpmPath: npmPath.root, localNpmPath: npmPath.local, rootNpxPath: npxPath.root, diff --git a/lib/content/index.js b/lib/content/index.js index 8b7c1d5c..54af4cf6 100644 --- a/lib/content/index.js +++ b/lib/content/index.js @@ -159,6 +159,7 @@ module.exports = { npm: 'npm', npx: 'npx', npmSpec: 'latest', + updateNpm: true, dependabot: 'increase-if-necessary', unwantedPackages: [ 'eslint', From b83a19a98ec5db85b96e645916dcb65dbc64fbbd Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 12:03:25 -0700 Subject: [PATCH 03/11] feat: add config option to disable eslint --- lib/config.js | 8 ++++++++ lib/content/index.js | 11 +++++++++-- lib/content/pkg.json | 2 +- test/apply/eslint.js | 24 ++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 test/apply/eslint.js diff --git a/lib/config.js b/lib/config.js index 0c6333c3..e643fbab 100644 --- a/lib/config.js +++ b/lib/config.js @@ -251,6 +251,14 @@ const getFullConfig = async ({ derived.engines = pkgConfig.engines || engines } + if (!pkgConfig.eslint) { + derived.ignorePaths = derived.ignorePaths.filter(p => !p.includes('eslint')) + if (Array.isArray(pkgConfig.requiredPackages?.devDependencies)) { + pkgConfig.requiredPackages.devDependencies = + pkgConfig.requiredPackages.devDependencies.filter(p => !p.includes('eslint')) + } + } + const gitUrl = await getGitUrl(rootPkg.path) if (gitUrl) { derived.repository = { diff --git a/lib/content/index.js b/lib/content/index.js index 54af4cf6..1437e1be 100644 --- a/lib/content/index.js +++ b/lib/content/index.js @@ -85,7 +85,10 @@ const rootRepo = { // dir. so we might want to combine these const rootModule = { add: { - '.eslintrc.js': 'eslintrc.js', + '.eslintrc.js': { + file: 'eslintrc.js', + filter: (p) => p.config.eslint, + }, '.gitignore': 'gitignore', '.npmrc': 'npmrc', 'SECURITY.md': 'SECURITY.md', @@ -113,7 +116,10 @@ const workspaceRepo = { // Changes for each workspace but applied to the relative workspace dir const workspaceModule = { add: { - '.eslintrc.js': 'eslintrc.js', + '.eslintrc.js': { + file: 'eslintrc.js', + filter: (p) => p.config.eslint, + }, '.gitignore': 'gitignore', 'package.json': 'pkg.json', }, @@ -155,6 +161,7 @@ module.exports = { ciVersions: ['14.17.0', '14.x', '16.13.0', '16.x', '18.0.0', '18.x'], lockfile: false, codeowner: '@npm/cli-team', + eslint: true, publish: false, npm: 'npm', npx: 'npx', diff --git a/lib/content/pkg.json b/lib/content/pkg.json index 387626ef..631f5dbd 100644 --- a/lib/content/pkg.json +++ b/lib/content/pkg.json @@ -2,7 +2,7 @@ "author": "GitHub Inc.", "files": {{{ json distPaths }}}, "scripts": { - "lint": "eslint \"**/*.js\"", + "lint": "{{#if eslint}}eslint \"**/*.js\"{{else}}echo linting disabled{{/if}}", "postlint": "template-oss-check", "template-oss-apply": "template-oss-apply --force", "lintfix": "{{ localNpmPath }} run lint -- --fix", diff --git a/test/apply/eslint.js b/test/apply/eslint.js new file mode 100644 index 00000000..56692818 --- /dev/null +++ b/test/apply/eslint.js @@ -0,0 +1,24 @@ +const t = require('tap') +const setup = require('../setup.js') + +t.test('can disable eslint', async (t) => { + const s = await setup(t, { + package: { + templateOSS: { + eslint: false, + }, + }, + }) + await s.apply() + + const pkg = await s.readJson('package.json') + delete pkg.templateOSS // templateOSS config has eslint in it + t.notMatch(JSON.stringify(pkg), 'eslint') + + const gitignore = await s.readFile('.gitignore') + t.notMatch(gitignore, 'eslint') + + const checks = await s.check() + t.equal(checks.length, 1) + t.notMatch(checks[0].solution, 'eslint') +}) From 6e02268f12cc65da2180527a6a18f5d737bab6c3 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 12:18:19 -0700 Subject: [PATCH 04/11] feat: allow adding latest to other ci versions --- lib/config.js | 8 +++++--- test/apply/engines.js | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/config.js b/lib/config.js index e643fbab..0b2cb1b6 100644 --- a/lib/config.js +++ b/lib/config.js @@ -228,9 +228,11 @@ const getFullConfig = async ({ if (pkgConfig.ciVersions) { let versions = pkgConfig.ciVersions - if (versions === 'latest') { - const defaultVersions = [rootPkgConfig, defaultConfig].find(c => Array.isArray(c.ciVersions)) - versions = defaultVersions.ciVersions.slice(-1) + if (versions === 'latest' || (Array.isArray(versions) && versions.includes('latest'))) { + const { ciVersions } = [isWorkspace ? rootPkgConfig : {}, defaultConfig] + .find(c => Array.isArray(c.ciVersions)) + const defaultLatest = ciVersions[ciVersions.length - 1] + versions = [].concat(versions).map(v => v === 'latest' ? defaultLatest : v) } const { targets, engines } = parseCIVersions(versions) diff --git a/test/apply/engines.js b/test/apply/engines.js index c907e66b..d34246ea 100644 --- a/test/apply/engines.js +++ b/test/apply/engines.js @@ -20,6 +20,26 @@ t.test('can set engines and ci separately', async (t) => { t.notOk(ci.includes('- 12')) }) +t.test('can set ci to latest plus other versions', async (t) => { + const s = await setup(t, { + package: { + templateOSS: { + ciVersions: ['6', '8', 'latest'], + engines: '*', + }, + }, + }) + await s.apply() + + const pkg = await s.readJson('package.json') + const ci = await s.readFile(join('.github', 'workflows', 'ci.yml')) + + t.equal(pkg.engines.node, '*') + t.ok(ci.includes('- 6')) + t.ok(ci.includes('- 8')) + t.ok(ci.includes('- 18.x')) +}) + t.test('latest ci versions', async (t) => { const s = await setup(t, { package: { From 83f36992073cb68b33c9b9e1dd90219b79671255 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 5 Jul 2023 20:12:02 +0000 Subject: [PATCH 05/11] chore: release 4.16.0 --- .release-please-manifest.json | 2 +- CHANGELOG.md | 9 +++++++++ package.json | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.release-please-manifest.json b/.release-please-manifest.json index e5a652cc..31b940b8 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "4.15.1" + ".": "4.16.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ab7ebcb..a2523e6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [4.16.0](https://github.com/npm/template-oss/compare/v4.15.1...v4.16.0) (2023-07-05) + +### Features + +* [`6e02268`](https://github.com/npm/template-oss/commit/6e02268f12cc65da2180527a6a18f5d737bab6c3) [#321](https://github.com/npm/template-oss/pull/321) allow adding latest to other ci versions (@lukekarrys) +* [`b83a19a`](https://github.com/npm/template-oss/commit/b83a19a98ec5db85b96e645916dcb65dbc64fbbd) [#321](https://github.com/npm/template-oss/pull/321) add config option to disable eslint (@lukekarrys) +* [`9606606`](https://github.com/npm/template-oss/commit/9606606f0163d56b785620881d5b01985f9218e3) [#321](https://github.com/npm/template-oss/pull/321) add config option to not update npm (@lukekarrys) +* [`73d7bf1`](https://github.com/npm/template-oss/commit/73d7bf174d34c5d51b87f937daf52b94c9e803c4) [#321](https://github.com/npm/template-oss/pull/321) add release/v branches to all branch CI (@lukekarrys) + ## [4.15.1](https://github.com/npm/template-oss/compare/v4.15.0...v4.15.1) (2023-05-03) ### Bug Fixes diff --git a/package.json b/package.json index 0be5f3ef..ba4c320a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@npmcli/template-oss", - "version": "4.15.1", + "version": "4.16.0", "description": "templated files used in npm CLI team oss projects", "main": "lib/content/index.js", "bin": { From 2a5cd532ce26b4fe66a6cb5a8b195c77e2520929 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 6 Jul 2023 14:40:42 +0000 Subject: [PATCH 06/11] deps: bump @npmcli/package-json from 3.1.1 to 4.0.0 Bumps [@npmcli/package-json](https://github.com/npm/package-json) from 3.1.1 to 4.0.0. - [Release notes](https://github.com/npm/package-json/releases) - [Changelog](https://github.com/npm/package-json/blob/main/CHANGELOG.md) - [Commits](https://github.com/npm/package-json/compare/v3.1.1...v4.0.0) --- updated-dependencies: - dependency-name: "@npmcli/package-json" dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ba4c320a..7fb1c9e1 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "@npmcli/arborist": "^6.0.0", "@npmcli/git": "^4.0.0", "@npmcli/map-workspaces": "^3.0.0", - "@npmcli/package-json": "^3.0.0", + "@npmcli/package-json": "^4.0.0", "@octokit/rest": "^19.0.4", "diff": "^5.0.0", "glob": "^10.1.0", From 7300da491ec1e596c5ad82434f7975a5e5a70f23 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 6 Jul 2023 15:22:55 -0700 Subject: [PATCH 07/11] fix: apply settings to all requested branches --- .github/settings.yml | 26 ++++++ .github/workflows/release.yml | 2 +- lib/content/release.yml | 2 +- lib/content/settings.yml | 4 +- .../test/apply/source-snapshots.js.test.cjs | 84 ++++++++++++++++++- 5 files changed, 112 insertions(+), 6 deletions(-) diff --git a/.github/settings.yml b/.github/settings.yml index 107aa0ad..adbef7e6 100644 --- a/.github/settings.yml +++ b/.github/settings.yml @@ -24,3 +24,29 @@ branches: apps: [] users: [] teams: [ "cli-team" ] + - name: latest + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] + - name: release/v* + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 810e7f3e..339dd69e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -78,7 +78,7 @@ jobs: let commentId = comments.find(c => c.user.login === 'github-actions[bot]' && c.body.startsWith(body))?.id body += `Release workflow run: ${workflow.html_url}\n\n#### Force CI to Update This Release\n\n` - body += `This PR will be updated and CI will run for every non-\`chore:\` commit that is pushed to \`main\`. ` + body += `This PR will be updated and CI will run for every non-\`chore:\` commit that is pushed to \`${REF_NAME}\`. ` body += `To force CI to update this PR, run this command:\n\n` body += `\`\`\`\ngh workflow run release.yml -r ${REF_NAME} -R ${owner}/${repo} -f release-pr=${issue_number}\n\`\`\`` diff --git a/lib/content/release.yml b/lib/content/release.yml index 7a8ea050..9f5f5f97 100644 --- a/lib/content/release.yml +++ b/lib/content/release.yml @@ -54,7 +54,7 @@ jobs: let commentId = comments.find(c => c.user.login === 'github-actions[bot]' && c.body.startsWith(body))?.id body += `Release workflow run: ${workflow.html_url}\n\n#### Force CI to Update This Release\n\n` - body += `This PR will be updated and CI will run for every non-\`chore:\` commit that is pushed to \`{{ defaultBranch }}\`. ` + body += `This PR will be updated and CI will run for every non-\`chore:\` commit that is pushed to \`${REF_NAME}\`. ` body += `To force CI to update this PR, run this command:\n\n` body += `\`\`\`\ngh workflow run release.yml -r ${REF_NAME} -R ${owner}/${repo} -f release-pr=${issue_number}\n\`\`\`` diff --git a/lib/content/settings.yml b/lib/content/settings.yml index 0f90cec6..70841282 100644 --- a/lib/content/settings.yml +++ b/lib/content/settings.yml @@ -9,7 +9,8 @@ repository: enable_vulnerability_alerts: true branches: - - name: {{ defaultBranch }} + {{#each branches}} + - name: {{ . }} protection: required_status_checks: null enforce_admins: true @@ -22,3 +23,4 @@ branches: apps: [] users: [] teams: ["cli-team"] + {{/each}} diff --git a/tap-snapshots/test/apply/source-snapshots.js.test.cjs b/tap-snapshots/test/apply/source-snapshots.js.test.cjs index bf8ea121..c20c3258 100644 --- a/tap-snapshots/test/apply/source-snapshots.js.test.cjs +++ b/tap-snapshots/test/apply/source-snapshots.js.test.cjs @@ -191,6 +191,32 @@ branches: apps: [] users: [] teams: [ "cli-team" ] + - name: latest + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] + - name: release/v* + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] .github/workflows/audit.yml ======================================== @@ -866,7 +892,7 @@ jobs: let commentId = comments.find(c => c.user.login === 'github-actions[bot]' && c.body.startsWith(body))?.id body += \`Release workflow run: \${workflow.html_url}/n/n#### Force CI to Update This Release/n/n\` - body += \`This PR will be updated and CI will run for every non-/\`chore:/\` commit that is pushed to /\`main/\`. \` + body += \`This PR will be updated and CI will run for every non-/\`chore:/\` commit that is pushed to /\`\${REF_NAME}/\`. \` body += \`To force CI to update this PR, run this command:/n/n\` body += \`/\`/\`/\`/ngh workflow run release.yml -r \${REF_NAME} -R \${owner}/\${repo} -f release-pr=\${issue_number}/n/\`/\`/\`\` @@ -1630,6 +1656,32 @@ branches: apps: [] users: [] teams: [ "cli-team" ] + - name: latest + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] + - name: release/v* + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] .github/workflows/audit.yml ======================================== @@ -2541,7 +2593,7 @@ jobs: let commentId = comments.find(c => c.user.login === 'github-actions[bot]' && c.body.startsWith(body))?.id body += \`Release workflow run: \${workflow.html_url}/n/n#### Force CI to Update This Release/n/n\` - body += \`This PR will be updated and CI will run for every non-/\`chore:/\` commit that is pushed to /\`main/\`. \` + body += \`This PR will be updated and CI will run for every non-/\`chore:/\` commit that is pushed to /\`\${REF_NAME}/\`. \` body += \`To force CI to update this PR, run this command:/n/n\` body += \`/\`/\`/\`/ngh workflow run release.yml -r \${REF_NAME} -R \${owner}/\${repo} -f release-pr=\${issue_number}/n/\`/\`/\`\` @@ -3347,6 +3399,32 @@ branches: apps: [] users: [] teams: [ "cli-team" ] + - name: latest + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] + - name: release/v* + protection: + required_status_checks: null + enforce_admins: true + required_pull_request_reviews: + required_approving_review_count: 1 + require_code_owner_reviews: true + require_last_push_approval: true + dismiss_stale_reviews: true + restrictions: + apps: [] + users: [] + teams: [ "cli-team" ] .github/workflows/ci-a.yml ======================================== @@ -4056,7 +4134,7 @@ jobs: let commentId = comments.find(c => c.user.login === 'github-actions[bot]' && c.body.startsWith(body))?.id body += \`Release workflow run: \${workflow.html_url}/n/n#### Force CI to Update This Release/n/n\` - body += \`This PR will be updated and CI will run for every non-/\`chore:/\` commit that is pushed to /\`main/\`. \` + body += \`This PR will be updated and CI will run for every non-/\`chore:/\` commit that is pushed to /\`\${REF_NAME}/\`. \` body += \`To force CI to update this PR, run this command:/n/n\` body += \`/\`/\`/\`/ngh workflow run release.yml -r \${REF_NAME} -R \${owner}/\${repo} -f release-pr=\${issue_number}/n/\`/\`/\`\` From 449066e1917b5ff113aba396b12986f5db5d25da Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 19:09:39 -0700 Subject: [PATCH 08/11] fix: determine parser based on target filename --- lib/util/files.js | 2 +- lib/util/parser.js | 46 ++++++++++++++++++++++++++++++---------------- package.json | 1 + 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/lib/util/files.js b/lib/util/files.js index c18979bf..e0abb5e7 100644 --- a/lib/util/files.js +++ b/lib/util/files.js @@ -35,7 +35,7 @@ const getParsers = (dir, files, options) => { return new (parser(Parser.Parsers))(target, file, options, { clean }) } - return new (Parser(file))(target, file, options, { clean }) + return new (Parser(target))(target, file, options, { clean }) }) return parsers.filter(Boolean) diff --git a/lib/util/parser.js b/lib/util/parser.js index e209dfe7..48ba7377 100644 --- a/lib/util/parser.js +++ b/lib/util/parser.js @@ -1,11 +1,12 @@ const fs = require('fs/promises') -const { basename, extname, dirname } = require('path') +const { dirname } = require('path') const yaml = require('yaml') const NpmPackageJson = require('@npmcli/package-json') const jsonParse = require('json-parse-even-better-errors') const Diff = require('diff') const { unset } = require('lodash') const ini = require('ini') +const { minimatch } = require('minimatch') const template = require('./template.js') const jsonDiff = require('./json-diff') const merge = require('./merge.js') @@ -167,17 +168,17 @@ class Base { } class Gitignore extends Base { - static types = ['codeowners', 'gitignore'] + static types = ['codeowners', '.gitignore'] comment = (c) => `# ${c}` } class Js extends Base { - static types = ['js'] + static types = ['*.js'] comment = (c) => `/* ${c} */` } class Ini extends Base { - static types = ['ini'] + static types = ['*.ini'] comment = (c) => `; ${c}` toString (s) { @@ -202,17 +203,17 @@ class Ini extends Base { } class IniMerge extends Ini { - static types = ['npmrc'] + static types = ['.npmrc'] merge = (t, s) => merge(t, s) } class Markdown extends Base { - static types = ['md'] + static types = ['*.md'] comment = (c) => `` } class Yml extends Base { - static types = ['yml'] + static types = ['*.yml'] comment = (c) => ` ${c}` toString (s) { @@ -274,7 +275,7 @@ class YmlMerge extends Yml { } class Json extends Base { - static types = ['json'] + static types = ['*.json'] // its a json comment! not really but we do add a special key // to json objects comment = (c) => ({ [`//${this.options.config.__NAME__}`]: c }) @@ -306,7 +307,7 @@ class JsonMerge extends Json { } class PackageJson extends JsonMerge { - static types = ['pkg.json'] + static types = ['package.json'] async prepare (s, t) { // merge new source with current pkg content @@ -348,15 +349,28 @@ const Parsers = { PackageJson, } -const parserLookup = Object.values(Parsers) +// Create an order to lookup parsers based on filename the only important part +// of ordering is that we want to match types by exact match first, then globs, +// so we always sort globs to the bottom +const parserLookup = [] +for (const parser of Object.values(Parsers)) { + for (const type of parser.types) { + const parserEntry = [type, parser] + if (type.includes('*')) { + parserLookup.push(parserEntry) + } else { + parserLookup.unshift(parserEntry) + } + } +} const getParser = (file) => { - const base = basename(file).toLowerCase() - const ext = extname(file).slice(1).toLowerCase() - - return parserLookup.find((p) => p.types.includes(base)) - || parserLookup.find((p) => p.types.includes(ext)) - || Parsers.Base + for (const [type, parser] of parserLookup) { + if (minimatch(file, type, { nocase: true, dot: true, matchBase: true })) { + return parser + } + } + return Parsers.Base } module.exports = getParser diff --git a/package.json b/package.json index 7fb1c9e1..f6b22a91 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "just-deep-map-values": "^1.1.1", "just-diff": "^6.0.0", "lodash": "^4.17.21", + "minimatch": "^9.0.2", "npm-package-arg": "^10.0.0", "proc-log": "^3.0.0", "release-please": "npm:@npmcli/release-please@^14.2.6", From 4624d9c17e30b4783ef6bfc28a72603cc076f0e7 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 22:45:39 -0700 Subject: [PATCH 09/11] feat: add overwrite false property to added files By default template-oss allows for written files to be configured on a per-repo basis. This is helpful for different repos to create their own templated files and apply those. With this change, a repo can now set overwrite: false to a templated file and have those updates be applied after the default template-oss changes are made. --- lib/apply/apply-files.js | 2 +- lib/check/check-apply.js | 3 +- lib/config.js | 8 ++-- lib/util/files.js | 72 ++++++++++++++++++++++++--------- lib/util/merge.js | 75 +++++++++++++++++++++++++++++------ lib/util/parser.js | 2 +- test/apply/overwrite-false.js | 40 +++++++++++++++++++ 7 files changed, 165 insertions(+), 37 deletions(-) create mode 100644 test/apply/overwrite-false.js diff --git a/lib/apply/apply-files.js b/lib/apply/apply-files.js index af8034a2..b852b0a3 100644 --- a/lib/apply/apply-files.js +++ b/lib/apply/apply-files.js @@ -9,7 +9,7 @@ const run = async (dir, files, options) => { await rmEach(dir, rm, options, (f) => fs.rm(f)) log.verbose('apply-files', 'add', add) - await parseEach(dir, add, options, (p) => p.applyWrite()) + await parseEach(dir, add, options, {}, (p) => p.applyWrite()) } module.exports = [{ diff --git a/lib/check/check-apply.js b/lib/check/check-apply.js index c76399bb..13b882e9 100644 --- a/lib/check/check-apply.js +++ b/lib/check/check-apply.js @@ -12,7 +12,8 @@ const run = async (type, dir, files, options) => { const { add: addFiles, rm: rmFiles } = files const rm = await rmEach(dir, rmFiles, options, (f) => rel(f)) - const [add, update] = partition(await parseEach(dir, addFiles, options, async (p) => { + const parseOpts = { allowMultipleSources: false } + const [add, update] = partition(await parseEach(dir, addFiles, options, parseOpts, async (p) => { const diff = await p.applyDiff() const target = rel(p.target) if (diff === null) { diff --git a/lib/config.js b/lib/config.js index 0b2cb1b6..d5acc35a 100644 --- a/lib/config.js +++ b/lib/config.js @@ -4,8 +4,8 @@ const semver = require('semver') const parseCIVersions = require('./util/parse-ci-versions.js') const getGitUrl = require('./util/get-git-url.js') const gitignore = require('./util/gitignore.js') -const { withArrays } = require('./util/merge.js') -const { FILE_KEYS, parseConfig: parseFiles, getAddedFiles } = require('./util/files.js') +const { mergeWithArrays } = require('./util/merge.js') +const { FILE_KEYS, parseConfig: parseFiles, getAddedFiles, mergeFiles } = require('./util/files.js') const CONFIG_KEY = 'templateOSS' const getPkgConfig = (pkg) => pkg[CONFIG_KEY] || {} @@ -14,7 +14,7 @@ const { name: NAME, version: LATEST_VERSION } = require('../package.json') const MERGE_KEYS = [...FILE_KEYS, 'defaultContent', 'content'] const DEFAULT_CONTENT = require.resolve(NAME) -const merge = withArrays('branches', 'distPaths', 'allowPaths', 'ignorePaths') +const merge = mergeWithArrays('branches', 'distPaths', 'allowPaths', 'ignorePaths') const makePosix = (v) => v.split(win32.sep).join(posix.sep) const deglob = (v) => makePosix(v).replace(/[/*]+$/, '') @@ -120,7 +120,7 @@ const getFullConfig = async ({ // Files get merged in from the default content (that template-oss provides) as well // as any content paths provided from the root or the workspace const fileDirs = uniq([useDefault && defaultDir, rootDir, pkgDir].filter(Boolean)) - const files = merge(useDefault && defaultFiles, rootFiles, pkgFiles) + const files = mergeFiles(useDefault && defaultFiles, rootFiles, pkgFiles) const repoFiles = isRoot ? files.rootRepo : files.workspaceRepo const moduleFiles = isRoot ? files.rootModule : files.workspaceModule diff --git a/lib/util/files.js b/lib/util/files.js index e0abb5e7..3b2b5723 100644 --- a/lib/util/files.js +++ b/lib/util/files.js @@ -1,27 +1,62 @@ const { join } = require('path') -const { defaultsDeep } = require('lodash') -const merge = require('./merge.js') +const { defaultsDeep, omit } = require('lodash') const deepMapValues = require('just-deep-map-values') const { glob } = require('glob') +const { mergeWithCustomizers, customizers } = require('./merge.js') const Parser = require('./parser.js') const template = require('./template.js') +const ADD_KEY = 'add' +const RM_KEY = 'rm' const FILE_KEYS = ['rootRepo', 'rootModule', 'workspaceRepo', 'workspaceModule'] const globify = pattern => pattern.split('\\').join('/') -const fileEntries = (dir, files, options) => Object.entries(files) - // remove any false values - .filter(([_, v]) => v !== false) - // target paths need to be joinsed with dir and templated - .map(([k, source]) => { - const target = join(dir, template(k, options)) - return [target, source] - }) +const mergeFiles = mergeWithCustomizers((value, srcValue, key, target, source, stack) => { + // This will merge all files except if the src file has overwrite:false. Then + // the files will be turned into an array so they can be applied on top of + // each other in the parser. + if ( + stack[0] === ADD_KEY && + FILE_KEYS.includes(stack[1]) && + value?.file && + srcValue?.overwrite === false + ) { + return [value, omit(srcValue, 'overwrite')] + } +}, customizers.overwriteArrays) + +const fileEntries = (dir, files, options, { allowMultipleSources = true } = {}) => { + const results = [] + + for (const [key, source] of Object.entries(files)) { + // remove any false values first since that means those targets are skipped + if (source === false) { + continue + } + + // target paths need to be joinsed with dir and templated + const target = join(dir, template(key, options)) + + if (Array.isArray(source)) { + // When turning an object of files into all its entries, we allow + // multiples when applying changes, but not when checking for changes + // since earlier files would always return as needing an update. So we + // either allow multiples and return the array or only return the last + // source file in the array. + const sources = allowMultipleSources ? source : source.slice(-1) + results.push(...sources.map(s => [target, s])) + } else { + results.push([target, source]) + } + } + + return results +} // given an obj of files, return the full target/source paths and associated parser -const getParsers = (dir, files, options) => { - const parsers = fileEntries(dir, files, options).map(([target, source]) => { +const getParsers = (dir, files, options, parseOptions) => { + const parsers = fileEntries(dir, files, options, parseOptions).map(([target, source]) => { const { file, parser, filter, clean: shouldClean } = source if (typeof filter === 'function' && !filter(options)) { @@ -62,9 +97,9 @@ const rmEach = async (dir, files, options, fn) => { return res.filter(Boolean) } -const parseEach = async (dir, files, options, fn) => { +const parseEach = async (dir, files, options, parseOptions, fn) => { const res = [] - for (const parser of getParsers(dir, files, options)) { + for (const parser of getParsers(dir, files, options, parseOptions)) { res.push(await fn(parser)) } return res.filter(Boolean) @@ -72,7 +107,7 @@ const parseEach = async (dir, files, options, fn) => { const parseConfig = (files, dir, overrides) => { const normalizeFiles = (v) => deepMapValues(v, (value, key) => { - if (key === 'rm' && Array.isArray(value)) { + if (key === RM_KEY && Array.isArray(value)) { return value.reduce((acc, k) => { acc[k] = true return acc @@ -88,16 +123,16 @@ const parseConfig = (files, dir, overrides) => { return value }) - const merged = merge(normalizeFiles(files), normalizeFiles(overrides)) + const merged = mergeFiles(normalizeFiles(files), normalizeFiles(overrides)) const withDefaults = defaultsDeep(merged, FILE_KEYS.reduce((acc, k) => { - acc[k] = { add: {}, rm: {} } + acc[k] = { [ADD_KEY]: {}, [RM_KEY]: {} } return acc }, {})) return withDefaults } -const getAddedFiles = (files) => files ? Object.keys(files.add || {}) : [] +const getAddedFiles = (files) => files ? Object.keys(files[ADD_KEY] || {}) : [] module.exports = { rmEach, @@ -105,4 +140,5 @@ module.exports = { FILE_KEYS, parseConfig, getAddedFiles, + mergeFiles, } diff --git a/lib/util/merge.js b/lib/util/merge.js index 90646b82..6395b925 100644 --- a/lib/util/merge.js +++ b/lib/util/merge.js @@ -1,21 +1,72 @@ -const { mergeWith } = require('lodash') +const { mergeWith: _mergeWith } = require('lodash') -const merge = (...objects) => mergeWith({}, ...objects, (value, srcValue, key) => { - if (Array.isArray(srcValue)) { - // Dont merge arrays, last array wins - return srcValue - } -}) +// Adapted from https://github.com/lodash/lodash/issues/3901#issuecomment-517983996 +// Allows us to keep track of the current key during each merge so a customizer +// can make different merges based on the parent keys. +const mergeWith = (...args) => { + const customizer = args.pop() + const objects = args + const sourceStack = [] + const keyStack = [] + return _mergeWith({}, ...objects, (value, srcValue, key, target, source) => { + let currentKeys + while (true) { + if (!sourceStack.length) { + sourceStack.push(source) + keyStack.push([]) + } + if (source === sourceStack[sourceStack.length - 1]) { + currentKeys = keyStack[keyStack.length - 1].concat(key) + sourceStack.push(srcValue) + keyStack.push(currentKeys) + break + } + sourceStack.pop() + keyStack.pop() + } + // Remove the last key since that is the current one and reverse the whole + // array so that the first entry is the parent, 2nd grandparent, etc + return customizer(value, srcValue, key, target, source, currentKeys.slice(0, -1).reverse()) + }) +} + +// Create a merge function that will run a set of customizer functions +const mergeWithCustomizers = (...customizers) => { + return (...objects) => mergeWith({}, ...objects, (...args) => { + for (const customizer of customizers) { + const result = customizer(...args) + // undefined means the customizer will defer to the next one + // the default behavior of undefined in lodash is to merge + if (result !== undefined) { + return result + } + } + }) +} -const mergeWithArrays = (...keys) => - (...objects) => mergeWith({}, ...objects, (value, srcValue, key) => { +const customizers = { + // Dont merge arrays, last array wins + overwriteArrays: (value, srcValue) => { + if (Array.isArray(srcValue)) { + return srcValue + } + }, + // Merge arrays if their key matches one of the passed in keys + mergeArrays: (...keys) => (value, srcValue, key) => { if (Array.isArray(srcValue)) { if (keys.includes(key)) { return (Array.isArray(value) ? value : []).concat(srcValue) } return srcValue } - }) + }, +} -module.exports = merge -module.exports.withArrays = mergeWithArrays +module.exports = { + // default merge is to overwrite arrays + merge: mergeWithCustomizers(customizers.overwriteArrays), + mergeWithArrays: (...keys) => mergeWithCustomizers(customizers.mergeArrays(...keys)), + mergeWithCustomizers, + mergeWith, + customizers, +} diff --git a/lib/util/parser.js b/lib/util/parser.js index 48ba7377..3ca63e9e 100644 --- a/lib/util/parser.js +++ b/lib/util/parser.js @@ -9,7 +9,7 @@ const ini = require('ini') const { minimatch } = require('minimatch') const template = require('./template.js') const jsonDiff = require('./json-diff') -const merge = require('./merge.js') +const { merge } = require('./merge.js') const setFirst = (first, rest) => ({ ...first, ...rest }) diff --git a/test/apply/overwrite-false.js b/test/apply/overwrite-false.js new file mode 100644 index 00000000..ca0bedbd --- /dev/null +++ b/test/apply/overwrite-false.js @@ -0,0 +1,40 @@ +const t = require('tap') +const setup = require('../setup.js') + +t.test('json merge', async (t) => { + const s = await setup(t, { + ok: true, + package: { + templateOSS: { + content: 'content', + }, + }, + testdir: { + content: { + 'index.js': `module.exports=${JSON.stringify({ + rootModule: { + add: { + 'package.json': { + file: 'more-package.json', + overwrite: false, + }, + }, + }, + })}`, + 'more-package.json': JSON.stringify({ + scripts: { + test: 'tap test/', + }, + }), + }, + }, + }) + + await s.apply() + + const pkg = await s.readJson('package.json') + t.equal(pkg.scripts.test, 'tap test/') + t.equal(pkg.scripts.snap, 'tap') + + t.strictSame(await s.check(), []) +}) From 710c25e404df0f5be2ef1a487704e7235db3c348 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 6 Jul 2023 15:52:16 -0700 Subject: [PATCH 10/11] fix: do not add dependabot files when config is falsy --- lib/content/index.js | 2 ++ test/apply/dependabot.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 test/apply/dependabot.js diff --git a/lib/content/index.js b/lib/content/index.js index 1437e1be..185ed46f 100644 --- a/lib/content/index.js +++ b/lib/content/index.js @@ -37,6 +37,7 @@ const sharedRootAdd = (name) => ({ // dependabot '.github/dependabot.yml': { file: 'dependabot.yml', + filter: (p) => p.config.dependabot, clean: (p) => p.config.isRoot, // dependabot takes a single top level config file. this parser // will run for all configured packages and each one will have @@ -48,6 +49,7 @@ const sharedRootAdd = (name) => ({ }, '.github/workflows/post-dependabot.yml': { file: 'post-dependabot.yml', + filter: (p) => p.config.dependabot, }, '.github/settings.yml': { file: 'settings.yml', diff --git a/test/apply/dependabot.js b/test/apply/dependabot.js new file mode 100644 index 00000000..2a0fdd01 --- /dev/null +++ b/test/apply/dependabot.js @@ -0,0 +1,30 @@ +const t = require('tap') +const setup = require('../setup.js') + +t.test('default dependabot', async (t) => { + const s = await setup(t) + await s.apply() + + const dependabot = await s.readFile('.github/dependabot.yml') + const postDependabot = await s.stat('.github/workflows/post-dependabot.yml') + + t.match(dependabot, 'increase-if-necessary') + t.ok(postDependabot) +}) + +t.test('no dependabot', async (t) => { + const s = await setup(t, { + package: { + templateOSS: { + dependabot: false, + }, + }, + }) + await s.apply() + + const dependabot = await s.stat('.github/dependabot.yml').catch(() => false) + const postDependabot = await s.stat('.github/workflows/post-dependabot.yml').catch(() => false) + + t.equal(dependabot, false) + t.equal(postDependabot, false) +}) From 102e1ae9d62646e888c71efe6d86dc491e5ca7ca Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 7 Jul 2023 15:31:01 +0000 Subject: [PATCH 11/11] chore: release 4.17.0 --- .release-please-manifest.json | 2 +- CHANGELOG.md | 16 ++++++++++++++++ package.json | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 31b940b8..ac650bfe 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "4.16.0" + ".": "4.17.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index a2523e6e..b0597fa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## [4.17.0](https://github.com/npm/template-oss/compare/v4.16.0...v4.17.0) (2023-07-07) + +### Features + +* [`4624d9c`](https://github.com/npm/template-oss/commit/4624d9c17e30b4783ef6bfc28a72603cc076f0e7) [#323](https://github.com/npm/template-oss/pull/323) add overwrite false property to added files (@lukekarrys) + +### Bug Fixes + +* [`710c25e`](https://github.com/npm/template-oss/commit/710c25e404df0f5be2ef1a487704e7235db3c348) [#327](https://github.com/npm/template-oss/pull/327) do not add dependabot files when config is falsy (@lukekarrys) +* [`449066e`](https://github.com/npm/template-oss/commit/449066e1917b5ff113aba396b12986f5db5d25da) [#323](https://github.com/npm/template-oss/pull/323) determine parser based on target filename (@lukekarrys) +* [`7300da4`](https://github.com/npm/template-oss/commit/7300da491ec1e596c5ad82434f7975a5e5a70f23) [#325](https://github.com/npm/template-oss/pull/325) apply settings to all requested branches (@lukekarrys) + +### Dependencies + +* [`2a5cd53`](https://github.com/npm/template-oss/commit/2a5cd532ce26b4fe66a6cb5a8b195c77e2520929) [#324](https://github.com/npm/template-oss/pull/324) bump @npmcli/package-json from 3.1.1 to 4.0.0 + ## [4.16.0](https://github.com/npm/template-oss/compare/v4.15.1...v4.16.0) (2023-07-05) ### Features diff --git a/package.json b/package.json index f6b22a91..0674d475 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@npmcli/template-oss", - "version": "4.16.0", + "version": "4.17.0", "description": "templated files used in npm CLI team oss projects", "main": "lib/content/index.js", "bin": {