fix(scope): Allow no scope in the commit message when validate is set to true.#69
Conversation
Codecov Report
@@ Coverage Diff @@
## master #69 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 121 123 +2
=====================================
+ Hits 121 123 +2
Continue to review full report at Codecov.
|
kentcdodds
left a comment
kentcdodds
left a comment
There was a problem hiding this comment.
Looks fine to me (just wrapping things in if (scopes.length > 0)). But I'm starting to not recognize the code in the codebase anymore. I'd like a review/merge from @spirosikmd and/or @Garbee
spirosikmd
left a comment
spirosikmd
left a comment
There was a problem hiding this comment.
Thank you very much for your PR! One comment from me. What do you think?
lib/validateMessage.js
Outdated
| if (scopes.length > 1) { | ||
| error('only one scope can be provided !'); | ||
| isValid = false; | ||
| if (scopes.length > 0) { |
There was a problem hiding this comment.
Maybe to avoid more if nesting at this point, we should inverse the check and return isValid? For example
if (!scopes) {
return isValid;
}There was a problem hiding this comment.
+1 to the early return as @spirosikmd recommended here. It is currently at 3 levels of if nesting, this makes 4 levels which is quite difficult to follow through. Having the early return keeps the nesting at 3 levels which is basically as minimal as we can get.
There was a problem hiding this comment.
That's a great suggestion. Let me quickly update it.
Garbee
left a comment
Garbee
left a comment
There was a problem hiding this comment.
Nice catch on the bug. I would like to see the new if be an early return instead of encapsulating the existing code.
lib/validateMessage.js
Outdated
| if (scopes.length > 1) { | ||
| error('only one scope can be provided !'); | ||
| isValid = false; | ||
| if (scopes.length > 0) { |
There was a problem hiding this comment.
+1 to the early return as @spirosikmd recommended here. It is currently at 3 levels of if nesting, this makes 4 levels which is quite difficult to follow through. Having the early return keeps the nesting at 3 levels which is basically as minimal as we can get.
|
Please take a look again and see if this is good. Thanks! 😃 |
|
Thanks @yeelan0319! |
5 participants
When
validate: true, it should allow the user provided no scope. Commit message such aschore: Publishshould pass.However, the current setup tries to call multiple method on
undefinedin that situation. So I fixed that by addingscope.length > 0before validate any individual message.