Skip to content

Conversation

byCedric
Copy link
Member

@byCedric byCedric commented Oct 7, 2018

Description

Fixes #463

Motivation and Context

See issue #463

Usage examples

See issue #463

How Has This Been Tested?

See issue #463

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@byCedric byCedric changed the title fix(cli): fix format module resolving fix(cli): import format module resolving Oct 7, 2018
@byCedric byCedric changed the title fix(cli): import format module resolving fix(cli): improve format module resolving Oct 7, 2018
Copy link
Contributor

@marionebl marionebl left a comment

Choose a reason for hiding this comment

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

This is great! I have a minor nitpick

return parserPreset.parserOpts;
}

function resolveModulePath(name, cwd) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could avoid the catch block via

function resolveModulePath(name, cwd) { return resolveFrom.silent(__dirname, name) || resolveFrom.silent(cwd, name) || resolveGlobal.silent(name); }
Copy link
Member Author

Choose a reason for hiding this comment

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

These are those "Doh!" moments for me 😅 I've no clue why I didn't think about this. I will change it now! Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! I've removed the extra resolveModulePath method. I used it for the try-catch contraption. Right now it's both easy to read, and the order is readable (at least, that's my opinion). 😄

@marionebl
Copy link
Contributor

@byCedric Have you pushed your latest changes?

@byCedric
Copy link
Member Author

@marionebl Now I have! 😅

@marionebl marionebl merged commit baed8b1 into conventional-changelog:master Oct 11, 2018
@marionebl
Copy link
Contributor

Thanks <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants