Skip to content

Commit

Permalink
[🔥AUDIT🔥] [argparsingfix] Fix issue where implicit args override expl…
Browse files Browse the repository at this point in the history
…icit config (#1986)

🖍 _This is an audit!_ 🖍

## Summary:
It turns out that we were overriding default values and explicit configuration with the default argument values for our boolean flags. This was down to minimist defaulting missing boolean args to `false` instead of not providing them. So, I've done a minimal transition to yargs instead.

In a future release, we can leverage more of yargs features, but for now, I've tried to keep things as similar as possible to previous releases.

However, given that this is a change in behaviour, I'm putting it out as a major release in case anyone was inadvertently relying on the broken processing of args and config.

## Test plan:
`yarn test`

Also, try running with a config where a boolean option is set to true and see that it is respected.

Author: somewhatabstract

Auditors: kevinbarabash

Required Reviewers:

Approved By:

Checks: ✅ CodeQL, ✅ Integration tests (windows-latest, 20.x), ✅ Test and build (macOS-latest, 20.x), ✅ Integration tests (macOS-latest, 20.x), ✅ Test and build (ubuntu-latest, 20.x), ✅ Integration tests (ubuntu-latest, 20.x), ✅ Test and build (windows-latest, 20.x), ❌ codecov/project, ✅ Lint and static types check (ubuntu-latest, 20.x), ✅ Analyze (javascript), ⏭️  dependabot, ✅ Update test coverage (ubuntu-latest, 20.x)

Pull Request URL: #1986
  • Loading branch information
somewhatabstract authored Feb 3, 2025
1 parent 1713a2f commit 6ed7f08
Show file tree
Hide file tree
Showing 9 changed files with 2,406 additions and 380 deletions.
5 changes: 5 additions & 0 deletions .changeset/healthy-onions-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"checksync": major
---

This fixes an issue where default argument values would override explicit configuration. This was unintentional; only explicitly provided argument values should override configuration. Since some folks may be relying on this broken behaviour, this is a major update. As part of this fix, argument parsing is now handled by yargs instead of minimist
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@
"@types/jest": "^29.5.12",
"@types/lodash": "^4.17.5",
"@types/micromatch": "^4.0.7",
"@types/minimist": "^1.2.5",
"@types/node": "^20.14.2",
"@types/parse-gitignore": "^1.0.2",
"@types/yargs": "^17.0.33",
"@typescript-eslint/eslint-plugin": "^7.13.0",
"@typescript-eslint/parser": "^7.13.0",
"adler-32": "^1.3.1",
Expand All @@ -81,13 +81,13 @@
"jest-extended": "^4.0.2",
"lodash": "^4.17.21",
"micromatch": "^4.0.7",
"minimist": "^1.2.8",
"prettier": "^3.3.2",
"rollup": "^4.18.0",
"rollup-plugin-copy": "^3.5.0",
"rollup-plugin-filesize": "^10.0.0",
"rollup-plugin-terser": "^7.0.2",
"typescript": "^5.4.5"
"typescript": "^5.4.5",
"yargs": "^17.7.2"
},
"resolutions": {
"kind-of": "^6.0.3",
Expand Down Expand Up @@ -126,4 +126,4 @@
"lint": "eslint --config eslint.config.mjs '{src,bin,__{tests,mocks}__}/**/*.ts'",
"publish:ci": "git diff --stat --exit-code HEAD && yarn build && changeset publish"
}
}
}
Loading

0 comments on commit 6ed7f08

Please sign in to comment.