- Notifications
You must be signed in to change notification settings - Fork319
Fix 132 tags not found#136
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix 132 tags not found#136
Uh oh!
There was an error while loading.Please reload this page.
Conversation
getLastTags should use pagination when the tag option wereset to 2 tags and these tags weren not found yet
codecov-io commentedFeb 19, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #136 +/- ##===========================================- Coverage 85.75% 38.69% -47.06%=========================================== Files 7 7 Lines 316 323 +7 ===========================================- Hits 271 125 -146- Misses 45 198 +153
Continue to review full report at Codecov.
|
@alexcanessa I don't understand why the codevcov tests are failing, do you have any clue? |
@wellDan28 I'm trying to understand. |
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.
@wellDan28 don't worry for the codecov, it's going to automatically get fixed whenmaster runs.
lib/src/Gren.js Outdated
| _validateRequiredTagsExists(tags,requireTags){ | ||
| if(requireTags.indexOf('all')>=0||!(requireTagsinstanceofArray))return; | ||
| consttagsNames=tags.map(x=>x.tag.name); |
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.
Please use explicit parameters name (i.e.tagData instead ofx)
lib/src/Gren.js Outdated
| if(missingTags.length>0){ | ||
| constnotFoundMessage=(missingTags.length===1) ?'tag is' :'tags are'; | ||
| throwchalk.red(`\nThe following${notFoundMessage} not found in the repository:${missingTags}. `+ | ||
| 'please provider tags that are exists in the repository'); |
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 not everything in the template literal?
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'm sorry, I don't understand, could you please explain what you mean?
lib/src/Gren.js Outdated
| if(missingTags.length>0){ | ||
| constnotFoundMessage=(missingTags.length===1) ?'tag is' :'tags are'; | ||
| throwchalk.red(`\nThe following${notFoundMessage} not found in the repository:${missingTags}. `+ | ||
| 'please provider tags that are exists in the repository'); |
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.
Please change so that:
please provider tags that are exists in the repositoryplease provide existing tags.
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.
@wellDan28 last feedback, promise 👍
lib/src/Gren.js Outdated
| constmissingTags=requireTags.filter(requireTag=>tagsNames.indexOf(requireTag)<0); | ||
| if(missingTags.length>0){ | ||
| constnotFoundMessage=(missingTags.length===1) ?'tag is' :'tags are'; |
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.
notFoundMessageinflection as it makes more sense 👍
lib/src/Gren.js Outdated
| * Check that the require tags are exists in tags | ||
| * | ||
| *@param {Array} tags | ||
| *@param {any} requireTags |
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.
You can expectrequireTags to beArray becausethis.options.tags it's always anArray
github-release-notes/lib/src/_utils.js
Lines 123 to 149 in3e6b2e4
| /** | |
| * Converts an array like string to an actual Array, | |
| * converting also underscores to spaces | |
| * | |
| *@since 0.6.0 | |
| *@public | |
| * | |
| *@param {string} arrayLike The string of items | |
| * e.g. | |
| * "wont_fix, duplicate, bug" | |
| * | |
| *@return {Array} The items with spaces instead of underscores. | |
| */ | |
| functionconvertStringToArray(arrayLike){ | |
| if(!arrayLike){ | |
| return[]; | |
| } | |
| if(typeofarrayLike==='object'){ | |
| returnObject.keys(arrayLike).map((itemKey)=>arrayLike[itemKey]); | |
| } | |
| returnarrayLike | |
| .replace(/\s/g,'') | |
| .split(',') | |
| .map((itemName)=>itemName.replace(/_/g,' ',itemName)); | |
| } |
Also, replace the@return with@throws as the function is only returningundefined
| if(missingTags.length>0){ | ||
| constnotFoundMessage=(missingTags.length===1) ?'tag is' :'tags are'; | ||
| throwchalk.red(`\nThe following${notFoundMessage} not found in the repository:${missingTags}. `+ | ||
| 'please provide existing tags.'); |
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 not one template literal?
@all-contributors please add@wellDan28 for bug |
I've put upa pull request to add@wellDan28! 🎉 |
@all-contributors please add@wellDan28 for code |
I've put upa pull request to add@wellDan28! 🎉 |
Close#132
The resolution contains two parts: