-
Notifications
You must be signed in to change notification settings - Fork 583
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 logging --version flag #1419
Conversation
🦋 Changeset detectedLatest commit: 919bcff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1419 +/- ##
=======================================
Coverage 81.02% 81.02%
=======================================
Files 54 54
Lines 2213 2213
Branches 654 659 +5
=======================================
Hits 1793 1793
Misses 416 416
Partials 4 4 ☔ View full report in Codecov by Sentry. |
// Version should only be shown if it's the only argument passed | ||
if (parsed.version && args.length === 1) { | ||
// eslint-disable-next-line import/no-extraneous-dependencies | ||
console.log(require("@changesets/cli/package.json").version); |
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.
would it work if we'd import it? IIRC Preconstruct (the tools that we use for building packages) uses the json
plugin. I'm hesitant when it comes to mixing import
s and require
s in the same files
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 copied this from below
changesets/packages/cli/src/index.ts
Lines 86 to 89 in b593756
- @changesets/cli@${ | |
// eslint-disable-next-line import/no-extraneous-dependencies | |
require("@changesets/cli/package.json").version | |
} |
I can give import a try though. The best case would also seem to be inlining the version directly in build-time
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.
It looks like default importing the package.json
works (named import not supported), so it would bundle the entire package.json
in the output.
Hey, checking in if this is ready to be merged now? I think the |
Fixes #1418
I didn't realize that
meow
would read the nearest package.json and auto support the--version
flag, so this PR adds that support manually.