-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
tests: fix parseReleases #265
Conversation
src/releases.js
Outdated
@@ -10,7 +10,7 @@ const parseReleases = async (tags, options, onParsed) => { | |||
const merges = commits.filter(commit => commit.merge).map(commit => commit.merge) | |||
const fixes = commits.filter(commit => commit.fixes).map(commit => ({ fixes: commit.fixes, commit })) | |||
|
|||
for (const plugin of options.plugins) { | |||
for (const plugin of Array.from(options.plugins || [])) { |
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.
why Array.from? what will it be if it's truthy but not an array?
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 suggested Array.from because it's the shortest way to check if plugins
is iterable. any other suggestion?
In that case the test might pass due to the conditions inside the loop.
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.
[].concat(options.plugins || [])
will be much faster, but also it's fine if options.plugins throws here when passed a truthy non-iterable value.
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 applied your suggestion, thanks!
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.
(by the "fine" part i mean it would be fine to have options.plugins || []
by itself. if it ends up non-iterable, it should throw)
Now all it needs is tests :-) |
@cookpete note that this has no tests yet |
Closes #264