Skip to content
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

Jump to bottom

feat: makes the list of metadata files configurable #425

Closed
wants to merge 1 commit into from
Closed

feat: makes the list of metadata files configurable #425

wants to merge 1 commit into from

Conversation

regseb
Copy link
Contributor

@regseb regseb commented Aug 2, 2019

  • Add CLI flag to configure metadata files (package.json, package-lock.json, ...): --pkg-files and --lock-files.
  • Add properties in configuration file to configure metadata files: pkgFiles and lockFiles.

@regseb regseb mentioned this pull request Aug 2, 2019
Closed
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 7e0739f on regseb:master into f34e39b on conventional-changelog:master.

@bcoe bcoe requested a review from jbottigliero August 6, 2019 01:50
@bcoe
Copy link
Member

bcoe commented Aug 6, 2019

@jbottigliero I think you were working on a similar patch for this?

@jbottigliero
Copy link
Member

jbottigliero commented Aug 7, 2019 •
edited
Loading

Hey @bcoe, @regseb – yes, this feature is included in #372.

In that PR, I've opted to keep the configuration names we're already using, but it should provide the same functionality.

@regseb do you have any thoughts on the implementation proposed in #372?

jbottigliero
jbottigliero reviewed Aug 7, 2019
View reviewed changes
test.js
@@ -822,7 +823,7 @@ describe('standard-version', function () {
shell.exec('git tag -a v1.0.0 -m "my awesome first release"')
commit('feat: new feature!')

require('./index')({ silent: true })
require('./index')(Object.assign({}, defaults, { silent: true }))
Copy link
Member

Choose a reason for hiding this comment

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

I believe manually importing and merging the defaults here is going to lead to false-positives in the tests.

@regseb
Copy link
Contributor Author

regseb commented Aug 7, 2019

You mean the PR #372? Your proposal includes my modifications with additional features. So you can close my PR.

I put two comments in the other PR: #372 (review)

@jbottigliero
Copy link
Member

Whoops, yes #372! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbottigliero jbottigliero jbottigliero left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@regseb @coveralls @bcoe @jbottigliero