Fix Lodash isArray deprecation #205

damassi · 2017-03-30T00:30:01Z

When installing the latest danger, I noticed this in the console output:

Replaces lodash.isarray with native implementation.

DangerCI · 2017-03-30T00:31:04Z

🎉

Generated by 🚫 dangerJS

codecov-io · 2017-03-30T00:32:39Z

Codecov Report

Merging #205 into master will increase coverage by 64.93%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master     #205       +/-   ##
===========================================
+ Coverage       0%   64.93%   +64.93%     
===========================================
  Files           4       32       +28     
  Lines         104      733      +629     
  Branches       16      102       +86     
===========================================
+ Hits            0      476      +476     
- Misses        104      257      +153

Impacted Files	Coverage Δ
source/platforms/github/GitHubGit.ts	`98.21% <100%> (ø)`
source/runner/Dangerfile.ts	`100% <0%> (ø)`
source/platforms/GitHub.ts	`37.2% <0%> (ø)`
source/platforms/platform.ts	`72.72% <0%> (ø)`
source/commands/utils/file-utils.ts	`100% <0%> (ø)`
source/runner/DangerUtils.ts	`100% <0%> (ø)`
source/api/fetch.ts	`14.28% <0%> (ø)`
source/ci_source/providers/index.ts	`100% <0%> (ø)`
source/ci_source/providers/Surf.ts	`64.7% <0%> (ø)`
source/ci_source/get_ci_source.ts	`79.16% <0%> (ø)`
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 749728c...6604d78. Read the comment docs.

macklinu

Thanks for your contribution, @damassi! I have a couple questions that I feel more confident letting @orta answer, but this change looks good to me. 🕺 🎉

macklinu · 2017-03-30T00:50:59Z

source/platforms/github/GitHubGit.ts

@@ -114,7 +113,7 @@ export default async function gitDSLForGitHub(api: GitHubAPI): Promise<GitDSL> {
      // added or removed. This makes it really easy to act on specific
      // changes to JSON DSLs

-      if (isarray(diff.after) && isarray(diff.before)) {
+      if (Array.isArray(diff.after) && Array.isArray(diff.before)) {


I think we should be safe using the native Array.isArray() instead of that deprecated lodash package. 👍

macklinu · 2017-03-30T00:51:29Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "danger",
-  "version": "0.14.1",
+  "version": "0.14.2",


Could you revert this change please? Unless we are good to ship this in a patch release - cc @orta

macklinu · 2017-03-30T00:53:31Z

changelog.md


 ### 0.14.1

+* Moved `@types/chalk` from dependencies to devDependencies - orta


Good call, I think this was shipped in 0.14.1.

macklinu · 2017-03-30T00:57:04Z

changelog.md

@@ -4,10 +4,11 @@

 ### 0.14.2

-* Moved `@types/chalk` from dependencies to devDependencies - orta
+* Replaced deprecated `lodash.isarray` package with `Array.isArray` - damassi


Looking for confirmation from @orta on shipping 0.14.2 or holding off on it for anything else to keep this changelog message here. Otherwise we usually add changelog messages right below the // Add your own contribution below line and then add the version number we are shipping to the changelog as we package up a release.

Ah, I see -- danger said to update the changelog and so I followed the format as it was there. It might be good to provide a little more direction in terms of version numbers, changelog, etc.

macklinu · 2017-03-30T01:03:50Z

Forgot to add, Danger commented on the PR requesting that yarn install is run (for dependency additions/removals to keep yarn.lock in sync with package.json). Could you please run that locally and push up a commit with the changes to yarn.lock? 🙂

damassi · 2017-03-30T03:04:26Z

Updated and rebased. It's odd -- I ran a yarn install first thing after pulling down the package, but I had to rm it and reinstall packages before it would update.

orta · 2017-03-30T07:56:31Z

Ah yeah, the version/changelog thing - again sorry about that, :D

I got stuck on a particularly long PR, which I thought I could get finished by the weekend ( #194 ) but inevitably I couldn't.

IMO it makes sense to be more verbose in the changelog about saying above the latest released version. 👍

orta · 2017-03-30T07:57:04Z

Shipping 0.14.2 sounds 👍

orta · 2017-03-30T08:01:55Z

@damassi with respect to the lockfile, you would have needed to run yarn remove lodash-isarray this would have removed the module then only re-created the dependency locks for anything related to it.

In deleting and re-creating it from scratch you've updated hundreds of dependencies in the stack, which is why it's a ~1000 LOC diff.

I'm ok with this, after eyeing it over, but ideally next time we should be a bit more strict about smaller lock file changes.

orta · 2017-03-30T08:40:38Z

Alright, I've shipped that - and all other PRs as 0.14.2

I've also updated the notice on the CHANGELOG to be more specific in fde4c54

damassi · 2017-03-30T18:41:42Z

Ah sorry about that! I wasn't aware of yarn remove <package>. Noted 👍

damassi force-pushed the master branch from 1152459 to 222b603 Compare March 30, 2017 00:44

macklinu reviewed Mar 30, 2017

View reviewed changes

Fix lodash isArray deprecation

6604d78

damassi force-pushed the master branch from 222b603 to 6604d78 Compare March 30, 2017 03:03

orta merged commit d388b29 into danger:master Mar 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Lodash isArray deprecation #205

Fix Lodash isArray deprecation #205

damassi commented Mar 30, 2017

DangerCI commented Mar 30, 2017 •

edited

Loading

codecov-io commented Mar 30, 2017 •

edited

Loading

macklinu left a comment

macklinu Mar 30, 2017

macklinu Mar 30, 2017

macklinu Mar 30, 2017

macklinu Mar 30, 2017

damassi Mar 30, 2017

macklinu commented Mar 30, 2017

damassi commented Mar 30, 2017

orta commented Mar 30, 2017

orta commented Mar 30, 2017

orta commented Mar 30, 2017 •

edited

Loading

orta commented Mar 30, 2017

damassi commented Mar 30, 2017


		### 0.14.1

		* Moved `@types/chalk` from dependencies to devDependencies - orta

Fix Lodash isArray deprecation #205

Fix Lodash isArray deprecation #205

Conversation

damassi commented Mar 30, 2017

DangerCI commented Mar 30, 2017 • edited Loading

codecov-io commented Mar 30, 2017 • edited Loading

Codecov Report

macklinu left a comment

Choose a reason for hiding this comment

macklinu Mar 30, 2017

Choose a reason for hiding this comment

macklinu Mar 30, 2017

Choose a reason for hiding this comment

macklinu Mar 30, 2017

Choose a reason for hiding this comment

macklinu Mar 30, 2017

Choose a reason for hiding this comment

damassi Mar 30, 2017

Choose a reason for hiding this comment

macklinu commented Mar 30, 2017

damassi commented Mar 30, 2017

orta commented Mar 30, 2017

orta commented Mar 30, 2017

orta commented Mar 30, 2017 • edited Loading

orta commented Mar 30, 2017

damassi commented Mar 30, 2017

DangerCI commented Mar 30, 2017 •

edited

Loading

codecov-io commented Mar 30, 2017 •

edited

Loading

orta commented Mar 30, 2017 •

edited

Loading