absolute-version / commit-and-tag-version

Fork of the excellent standard-version. Automate versioning and CHANGELOG generation, with semver.org and conventionalcommits.org :trophy:
ISC License
385 stars 36 forks source link

chore(dep): add stringify-package to project source #65

Closed Merfoo closed 1 year ago

Merfoo commented 1 year ago

Resolves https://github.com/absolute-version/commit-and-tag-version/issues/49

I considered replacing the stringify-package package with the @npmcli/package-json package as mentioned in https://github.com/absolute-version/commit-and-tag-version/issues/49#issuecomment-1360602538 but decided that it'd be less intrusive to this project's code to just copy over the code and tests over from the stringify-package package instead (https://github.com/npm/stringify-package).

I also thought that adding @npmcli/package-json as a dependency a little overkill for this project's singular use case of updating the package.json file's version field.

TimothyJones commented 1 year ago

Thanks for the PR!

One request - the package you've included is under the ISC license, which we must include if we are distributing a copy. Can you drop that in a comment in the source file, maybe alongside a link to the original?

Otherwise, I like your solution, especially as it's such a simple package - and, as you point out, we don't need all the features of the official npm one.

If ever they change the format and we need to do something more complicated to write to this field, we'd have to use the official one instead. But given that's an unlikely scenario (which would also have other far-reaching changes) paying that cost now seems unnecessary.

codecov-commenter commented 1 year ago

Codecov Report

Merging #65 (5b44502) into master (bc6f792) will increase coverage by 0.05%. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   97.52%   97.57%   +0.05%     
==========================================
  Files          29       31       +2     
  Lines        1253     1280      +27     
==========================================
+ Hits         1222     1249      +27     
  Misses         31       31              
Impacted Files Coverage Δ
lib/stringify-package.js 100.00% <100.00%> (ø)
lib/updaters/types/json.js 93.33% <100.00%> (ø)
test/stringify-package.spec.js 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

Merfoo commented 1 year ago

I've added the ISC as a comment to the source code and added a link to the stringify-package license as well as requested.

(FYI I'm impressed by the fast response to the PR 😄)

TimothyJones commented 1 year ago

Thanks! I try to respond as soon as I see it, especially if it's a small PR.

Releasing 11.2.1 now.