Skip to content

fix: Replace parse-git-config with simple-git #1485

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix: Replace parse-git-config with simple-git #1485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,14 @@
"override-require": "^1.1.1",
"p-limit": "^2.1.0",
"parse-diff": "^0.7.0",
"parse-git-config": "^2.0.3",
"parse-github-url": "^1.0.2",
"parse-link-header": "^2.0.0",
"pinpoint": "^1.1.0",
"prettyjson": "^1.2.1",
"readline-sync": "^1.4.9",
"regenerator-runtime": "^0.13.9",
"require-from-string": "^2.0.2",
"simple-git": "^3.27.0",
"supports-hyperlinks": "^1.0.1"
},
"husky": {
Expand Down
1 change: 0 additions & 1 deletion source/ambient.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ declare module "supports-hyperlinks"
// export default function(code: string, filename?: string, opts?: Partial<RequireOptions>): any
// }

declare module "parse-git-config"
declare module "parse-github-url"
// Basically does one thing
declare module "override-require"
Expand Down
2 changes: 1 addition & 1 deletion source/commands/danger-init.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface App {
const app: App = program as any

const go = async (app: App) => {
const state = generateInitialState(process)
const state = await generateInitialState(process)
const ui: InitUI = createUI(state, app)

if (!state.isGitHub) {
Expand Down
11 changes: 6 additions & 5 deletions source/commands/init/get-repo-slug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import parseGitConfig from "parse-git-config"
import { simpleGit } from "simple-git"
import parseGithubURL from "parse-github-url"

export const getRepoSlug = () => {
const config = parseGitConfig.sync()
const possibleRemotes = [config['remote "upstream"'], config['remote "origin"']].filter((f) => f)
export const getRepoSlug: () => Promise<string> = async () => {
const git = simpleGit()
const remotes = await git.getRemotes(true)
const possibleRemotes = remotes.filter((remote) => remote.name === "upstream" || remote.name === "origin")
if (possibleRemotes.length === 0) {
return null
}

const ghData = possibleRemotes.map((r) => parseGithubURL(r.url))
const ghData = possibleRemotes.map((r) => parseGithubURL(r.refs.fetch))
return ghData.length ? ghData[0].repo : undefined
}
4 changes: 2 additions & 2 deletions source/commands/init/state-setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const createUI = (state: InitState, app: any): InitUI => {
}
}

export const generateInitialState = (osProcess: NodeJS.Process): InitState => {
export const generateInitialState = async (osProcess: NodeJS.Process): Promise<InitState> => {
const isMac = osProcess.platform === "darwin"
const isWindows = osProcess.platform === "win32"
const folderName = capitalizeFirstLetter(camelCase(basename(osProcess.cwd())))
Expand All @@ -41,7 +41,7 @@ export const generateInitialState = (osProcess: NodeJS.Process): InitState => {
const hasGitHubActions = fs.existsSync(".github/") && fs.existsSync(".github/workflows")

const ciType = hasGitHubActions ? "gh-actions" : hasTravis ? "travis" : hasCircle ? "circle" : "unknown"
const repoSlug = getRepoSlug()
const repoSlug = await getRepoSlug()
const isGitHub = !!repoSlug

return {
Expand Down
77 changes: 22 additions & 55 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,18 @@
"@jridgewell/resolve-uri" "^3.1.0"
"@jridgewell/sourcemap-codec" "^1.4.14"

"@kwsites/file-exists@^1.1.1":
version "1.1.1"
resolved "https://registry.yarnpkg.com/@kwsites/file-exists/-/file-exists-1.1.1.tgz#ad1efcac13e1987d8dbaf235ef3be5b0d96faa99"
integrity sha512-m9/5YGR18lIwxSFDwfE3oA7bWuq9kdau6ugN4H2rJeyhFQZcG9AgSHkQtSD15a8WvTgfz9aikZMrKPHvbpqFiw==
dependencies:
debug "^4.1.1"

"@kwsites/promise-deferred@^1.1.1":
version "1.1.1"
resolved "https://registry.yarnpkg.com/@kwsites/promise-deferred/-/promise-deferred-1.1.1.tgz#8ace5259254426ccef57f3175bc64ed7095ed919"
integrity sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw==

"@nicolo-ribaudo/[email protected]":
version "2.1.8-no-fsevents.3"
resolved "https://registry.yarnpkg.com/@nicolo-ribaudo/chokidar-2/-/chokidar-2-2.1.8-no-fsevents.3.tgz#323d72dd25103d0c4fbdce89dadf574a787b1f9b"
Expand Down Expand Up @@ -3651,13 +3663,6 @@ expand-template@^2.0.3:
resolved "https://registry.yarnpkg.com/expand-template/-/expand-template-2.0.3.tgz#6e14b3fcee0f3a6340ecb57d2e8918692052a47c"
integrity sha512-XYfuKMvj4O35f/pOXLObndIRvyQ+/+6AhODh+OKWj9S9498pHHn/IMszH+gt0fBCRWMNfk1ZSp5x3AifmnI2vg==

expand-tilde@^2.0.2:
version "2.0.2"
resolved "https://registry.yarnpkg.com/expand-tilde/-/expand-tilde-2.0.2.tgz#97e801aa052df02454de46b02bf621642cdc8502"
integrity sha512-A5EmesHW6rfnZ9ysHQjPdJRni0SRar0tjtG5MNtm9n5TUvsYU8oozprtRD4AqHxcZWWlVuAmQo2nWKfN9oyjTw==
dependencies:
homedir-polyfill "^1.0.1"

expect@^28.0.0, expect@^28.1.3:
version "28.1.3"
resolved "https://registry.yarnpkg.com/expect/-/expect-28.1.3.tgz#90a7c1a124f1824133dd4533cce2d2bdcb6603ec"
Expand All @@ -3669,13 +3674,6 @@ expect@^28.0.0, expect@^28.1.3:
jest-message-util "^28.1.3"
jest-util "^28.1.3"

extend-shallow@^2.0.1:
version "2.0.1"
resolved "https://registry.yarnpkg.com/extend-shallow/-/extend-shallow-2.0.1.tgz#51af7d614ad9a9f610ea1bafbb989d6b1c56890f"
integrity sha512-zCnTtlxNoAiDc3gqY2aYAWFx7XWWiasuF2K8Me5WbN8otHKTUKBwjPtNpRs/rbUZm7KxWAaNj7P1a/p52GbVug==
dependencies:
is-extendable "^0.1.0"

external-editor@^3.1.0:
version "3.1.0"
resolved "https://registry.yarnpkg.com/external-editor/-/external-editor-3.1.0.tgz#cb03f740befae03ea4d283caed2741a83f335495"
Expand Down Expand Up @@ -3845,11 +3843,6 @@ fs-constants@^1.0.0:
resolved "https://registry.yarnpkg.com/fs-constants/-/fs-constants-1.0.0.tgz#6be0de9be998ce16af8afc24497b9ee9b7ccd9ad"
integrity sha512-y6OAwoSIf7FyjMIv94u+b5rdheZEjzR63GTyZJm5qh4Bi+2YgwLCcI/fPFZkL5PSixOt6ZNKm+w+Hfp/Bciwow==

fs-exists-sync@^0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/fs-exists-sync/-/fs-exists-sync-0.1.0.tgz#982d6893af918e72d08dec9e8673ff2b5a8d6add"
integrity sha512-cR/vflFyPZtrN6b38ZyWxpWdhlXrzZEBawlpBQMq7033xVY7/kg0GDMBK5jg8lDYQckdJ5x/YC88lM3C7VMsLg==

fs-extra@^4.0.0:
version "4.0.3"
resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-4.0.3.tgz#0d852122e5bc5beb453fb028e9c0c9bf36340c94"
Expand Down Expand Up @@ -3993,15 +3986,6 @@ get-uri@^6.0.1:
data-uri-to-buffer "^6.0.2"
debug "^4.3.4"

git-config-path@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/git-config-path/-/git-config-path-1.0.1.tgz#6d33f7ed63db0d0e118131503bab3aca47d54664"
integrity sha512-KcJ2dlrrP5DbBnYIZ2nlikALfRhKzNSX0stvv3ImJ+fvC4hXKoV+U+74SV0upg+jlQZbrtQzc0bu6/Zh+7aQbg==
dependencies:
extend-shallow "^2.0.1"
fs-exists-sync "^0.1.0"
homedir-polyfill "^1.0.0"

git-up@^8.0.0:
version "8.0.1"
resolved "https://registry.yarnpkg.com/git-up/-/git-up-8.0.1.tgz#2a82cfbc77b5eb04074ab1e48593911981654fc7"
Expand Down Expand Up @@ -4174,13 +4158,6 @@ highlight.js@^9.0.0:
resolved "https://registry.yarnpkg.com/highlight.js/-/highlight.js-9.18.5.tgz#d18a359867f378c138d6819edfc2a8acd5f29825"
integrity sha512-a5bFyofd/BHCX52/8i8uJkjr9DYwXIPnM/plwI6W7ezItLGqzt7X2G2nXuYSfsIJdkwwj/g9DG1LkcGJI/dDoA==

homedir-polyfill@^1.0.0, homedir-polyfill@^1.0.1:
version "1.0.3"
resolved "https://registry.yarnpkg.com/homedir-polyfill/-/homedir-polyfill-1.0.3.tgz#743298cef4e5af3e194161fbadcc2151d3a058e8"
integrity sha512-eSmmWE5bZTK2Nou4g0AI3zZ9rswp7GRKoKXS1BLUkvPviOqs4YTN1djQIqrXy9k5gEtdLPy86JjRwsNM9tnDcA==
dependencies:
parse-passwd "^1.0.0"

hosted-git-info@^2.1.4:
version "2.8.9"
resolved "https://registry.yarnpkg.com/hosted-git-info/-/hosted-git-info-2.8.9.tgz#dffc0bf9a21c02209090f2aa69429e1414daf3f9"
Expand Down Expand Up @@ -4334,7 +4311,7 @@ [email protected]:
resolved "https://registry.yarnpkg.com/ini/-/ini-4.1.1.tgz#d95b3d843b1e906e56d6747d5447904ff50ce7a1"
integrity sha512-QQnnxNyfvmHFIsj7gkPcYymR8Jdw/o7mp5ZFihxn6h8Ci6fh3Dx4E1gPjpQEpIuPo9XVNY/ZUwh4BPMjGyL01g==

ini@^1.3.4, ini@^1.3.5, ini@~1.3.0:
ini@^1.3.4, ini@~1.3.0:
version "1.3.8"
resolved "https://registry.yarnpkg.com/ini/-/ini-1.3.8.tgz#a29da425b48806f34767a4efce397269af28432c"
integrity sha512-JV/yugV2uzW5iMRSiZAyDtQd+nxtUnjeLt0acNdw98kKLrvuRVyB80tsREOE7yvGVgalhZ6RNXCmEHkUKBKxew==
Expand Down Expand Up @@ -4416,11 +4393,6 @@ is-docker@^3.0.0:
resolved "https://registry.yarnpkg.com/is-docker/-/is-docker-3.0.0.tgz#90093aa3106277d8a77a5910dbae71747e15a200"
integrity sha512-eljcgEDlEns/7AXFosB5K/2nCM4P7FQPkGc/DWLy5rmFEWvZayGrik1d9/QIY5nJ4f9YsVvBkA6kJpHn9rISdQ==

is-extendable@^0.1.0:
version "0.1.1"
resolved "https://registry.yarnpkg.com/is-extendable/-/is-extendable-0.1.1.tgz#62b110e289a471418e3ec36a617d472e301dfc89"
integrity sha512-5BMULNob1vgFX6EjQw5izWDxrecWK9AM72rugNr0TFldMOi0fj6Jk+zeKIt0xGj4cEfQIJth4w3OKWOJ4f+AFw==

is-extglob@^2.1.1:
version "2.1.1"
resolved "https://registry.yarnpkg.com/is-extglob/-/is-extglob-2.1.1.tgz#a88c02535791f02ed37c76a1b9ea9773c833f8c2"
Expand Down Expand Up @@ -5930,15 +5902,6 @@ parse-diff@^0.7.0:
resolved "https://registry.yarnpkg.com/parse-diff/-/parse-diff-0.7.1.tgz#9b7a2451c3725baf2c87c831ba192d40ee2237d4"
integrity sha512-1j3l8IKcy4yRK2W4o9EYvJLSzpAVwz4DXqCewYyx2vEwk2gcf3DBPqc8Fj4XV3K33OYJ08A8fWwyu/ykD/HUSg==

parse-git-config@^2.0.3:
version "2.0.3"
resolved "https://registry.yarnpkg.com/parse-git-config/-/parse-git-config-2.0.3.tgz#6fb840d4a956e28b971c97b33a5deb73a6d5b6bb"
integrity sha512-Js7ueMZOVSZ3tP8C7E3KZiHv6QQl7lnJ+OkbxoaFazzSa2KyEHqApfGbU3XboUgUnq4ZuUmskUpYKTNx01fm5A==
dependencies:
expand-tilde "^2.0.2"
git-config-path "^1.0.1"
ini "^1.3.5"

parse-github-url@^1.0.2:
version "1.0.3"
resolved "https://registry.yarnpkg.com/parse-github-url/-/parse-github-url-1.0.3.tgz#2ab55642c8685b63fbe2a196f5abe4ae9bd68abc"
Expand Down Expand Up @@ -5994,11 +5957,6 @@ parse-ms@^4.0.0:
resolved "https://registry.yarnpkg.com/parse-ms/-/parse-ms-4.0.0.tgz#c0c058edd47c2a590151a718990533fd62803df4"
integrity sha512-TXfryirbmq34y8QBwgqCVLi+8oA3oWx2eAnSn62ITyEhEYaWRlVZ2DvMM9eZbMs/RfxPu/PK/aBLyGj4IrqMHw==

parse-passwd@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/parse-passwd/-/parse-passwd-1.0.0.tgz#6d5b934a456993b23d37f40a382d6f1666a8e5c6"
integrity sha512-1Y1A//QUXEZK7YKz+rD9WydcE1+EuPr6ZBgKecAB8tmoW6UFv0NREVJe1p+jRxtThkcbbKkfwIbWJe/IeE6m2Q==

parse-path@^7.0.0:
version "7.0.1"
resolved "https://registry.yarnpkg.com/parse-path/-/parse-path-7.0.1.tgz#ae548cd36315fd8881a3610eae99fa08123ee0e2"
Expand Down Expand Up @@ -6858,6 +6816,15 @@ simple-get@^4.0.0:
once "^1.3.1"
simple-concat "^1.0.0"

simple-git@^3.27.0:
version "3.27.0"
resolved "https://registry.yarnpkg.com/simple-git/-/simple-git-3.27.0.tgz#f4b09e807bda56a4a3968f635c0e4888d3decbd5"
integrity sha512-ivHoFS9Yi9GY49ogc6/YAi3Fl9ROnF4VyubNylgCkA+RVqLaKWnDSzXOVzya8csELIaWaYNutsEuAhZrtOjozA==
dependencies:
"@kwsites/file-exists" "^1.1.1"
Copy link
Member

@fbartho fbartho Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Concerning dependency: no-commits in 5 years https://github.com/kwsites/file-exists & similarly to the promise-deferred library (see next line).

simple-git as a whole hasn’t had any commits in 4 years https://github.com/simple-git-js/simple-git/ (Updated, that comment was about wrong simple-git — I’m reassured that the correct simple-git has active maintenance)

It’s one thing if tools are based in a mature stack that doesn’t change. But all these tools are based on node & npm. And as we all now, that stack has been constantly changing. I’m not sure going from a package that last saw maintenance 6 years ago to one that is 4 years ago is a big enough improvement? (Also the debug library is a little weird)

My 2cents: I’d expect new libraries we adopt have a story about ESM (& CJS still), otherwise we’re piling up more work in front of maintaining danger-js in the next year or so.

Copy link
Member

@orta orta Apr 10, 2025

Choose a reason for hiding this comment

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

That's not the dependency: https://www.npmjs.com/package/simple-git is https://github.com/steveukx/git-js

Copy link
Member

@fbartho fbartho Apr 10, 2025

Choose a reason for hiding this comment

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

Oh!

Copy link
Member

@fbartho fbartho Apr 10, 2025

Choose a reason for hiding this comment

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

I’m much less worried about simple-git now, thanks @orta! My questions about the transitive deps are still there, but active maintenance on simple-git is at least reassuring

Copy link
Member

@orta orta Apr 10, 2025

Choose a reason for hiding this comment

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

For me, the biggest flaw in this PR is that parse-git-config did one simple thing in 200 lines - this new dependency is massive in comparison because it implements every git command and everyone has to get all this stuff so we can just look at a list of remotes in danger init

Why did you choose it? Did you look for a smaller dependency @ryanb93 ?

Copy link
Author

@ryanb93 ryanb93 Apr 11, 2025

Choose a reason for hiding this comment

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

Honestly, it just came up as the first library that could implement this functionality - but you're correct, it's a heavy dependency to bring in for only a single task. I'm not sure how you feel about using execSync but we could remove these dependencies entirely and use something like:

const remoteUrl = execSync('git config --get remote.origin.url').toString().trim();

Copy link
Member

@orta orta Apr 11, 2025

Choose a reason for hiding this comment

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

Yeah, I think that's pretty reasonable 👍🏻

"@kwsites/promise-deferred" "^1.1.1"
debug "^4.3.5"

sisteransi@^1.0.5:
version "1.0.5"
resolved "https://registry.yarnpkg.com/sisteransi/-/sisteransi-1.0.5.tgz#134d681297756437cc05ca01370d3a7a571075ed"
Expand Down
Loading