Merge pull request #1788 from artsy/orta-dm_update

Update Danger, make the Dangerfile use the new scoped syntax #trivial

Dangerfile

@@ -1,29 +1,32 @@
 # Sometimes its a README fix, or something like that - which isn't relevant for
 # including in a CHANGELOG for example
-declared_trivial = pr_title.include? "#trivial"
+declared_trivial = github.pr_title.include? "#trivial"
 
 # Just to let people know
-warn("PR is classed as Work in Progress") if pr_title.include? "[WIP]"
+warn("PR is classed as Work in Progress") if github.pr_title.include? "[WIP]"
 
 # Oi, CHANGELOGs please
-fail("No CHANGELOG changes made") if lines_of_code > 50 && !modified_files.include?("CHANGELOG.yml") && !declared_trivial
+fail("No CHANGELOG changes made") if git.lines_of_code > 50 && !github.modified_files.include?("CHANGELOG.yml") && !declared_trivial
 
 # Stop skipping some manual testing
-warn("Needs testing on a Phone if change is non-trivial") if lines_of_code > 50 && !pr_title.include?("📱")
+warn("Needs testing on a Phone if change is non-trivial") if git.lines_of_code > 50 && !pr_title.include?("📱")
 
 # Don't let testing shortcuts get into master
 fail("fdescribe left in tests") if `grep -r fdescribe Artsy_Tests/`.length > 1
+# Handle Objective-C and Swift
 fail("fit left in tests") if `grep -rI "fit(@" Artsy_Tests/`.length > 1
 fail("fit left in tests") if `grep -rI "fit(" Artsy_Tests/`.length > 1
 
 # Devs shouldn't ship changes to this file
-fail("Developer Specific file shouldn't be changed") if modified_files.include?("Artsy/View_Controllers/App_Navigation/ARTopMenuViewController+DeveloperExtras.m")
-fail("Developer Specific file shouldn't be changed") if modified_files.include?("Artsy/View_Controllers/App_Navigation/ARTopMenuViewController+SwiftDeveloperExtras.swift")
-fail("Developer Specific file shouldn't be changed") if modified_files.include?("Artsy.xcodeproj/xcshareddata/xcschemes/Artsy.xcscheme")
+["Artsy/View_Controllers/App_Navigation/ARTopMenuViewController+DeveloperExtras.m",
+ "Artsy/View_Controllers/App_Navigation/ARTopMenuViewController+SwiftDeveloperExtras.swift",
+ "Artsy.xcodeproj/xcshareddata/xcschemes/Artsy.xcscheme"].each do |dev_file|
+  fail("Developer Specific file shouldn't be changed") if git.modified_files.include? dev_file
+end
 
 # Did you make analytics changes? Well you should also include a change to our analytics spec
-made_analytics_changes = modified_files.include?("/Artsy/App/ARAppDelegate+Analytics.m")
-made_analytics_specs_changes = modified_files.include?("/Artsy_Tests/Analytics_Tests/ARAppAnalyticsSpec.m")
+made_analytics_changes = git.modified_files.include?("/Artsy/App/ARAppDelegate+Analytics.m")
+made_analytics_specs_changes = git.modified_files.include?("/Artsy_Tests/Analytics_Tests/ARAppAnalyticsSpec.m")
 if made_analytics_changes
     fail("Analytics changes should have reflected specs changes") if !made_analytics_specs_changes
 
@@ -32,32 +35,39 @@ if made_analytics_changes
     message('Also, double check the [Analytics Eigen schema](https://docs.google.com/spreadsheets/u/1/d/1bLbeOgVFaWzLSjxLOBDNOKs757-zBGoLSM1lIz3OPiI/edit#gid=497747862) if the changes are non-trivial.')
 end
 
+# It's relatively common that we ship a CHANGELOG which isn't valid YAML.
+# While it's a bit annoying, having the structure from the YAML is nice.
 
-# CHANGELOG should lint
 begin
   readme_yaml = File.read "CHANGELOG.yml"
   readme_data = YAML.load readme_yaml
+
   # Common error when making a new version
   fail "Upcoming is an array, it should be an object" if readme_data["upcoming"].is_a? Array  
+
   # Tie all releases to a date
   for release in readme_data["releases"]
     fail "Release #{release["version"]} does not have a date" unless release["date"]
   end 
 
 rescue StandardError
-  fail("CHANGELOG isn't legit YAML")
+  # YAML could not be parsed, fail the build.
+  fail("CHANGELOG isn't valid YAML")
 end
 
+# Use Circle's build artifacts feature to let Danger read the build, and test logs.
+# There's nothing fancy here, just a unix command chain with `tee` sending the output to a known file.
+#
 build_file = File.join(ENV["CIRCLE_ARTIFACTS"], "xcode_build_raw.log")
 test_file = File.join(ENV["CIRCLE_ARTIFACTS"], "xcode_test_raw.log")
 
-# So if there's snapshot fails, we should also fail danger, but we can make the thing clickable in a comment instead of hidden in the log
-# Note: this may break in a future build of Danger, I am debating sandboxing the runner from ENV vars.
+# If there's snapshot fails, we should also fail danger, but we can make the thing clickable in a comment instead of hidden in the log
+# Note: this _may_ break in a future build of Danger, I am debating sandboxing the runner from ENV vars.
 test_log = File.read test_file
 snapshots_url = test_log.match(%r{https://eigen-ci.s3.amazonaws.com/\d+/index.html})
 fail("There were [snapshot errors](#{snapshots_url})") if snapshots_url
 
-# Look for unstubbed networking requests, as these can be a source of test flakiness
+# Look for unstubbed networking requests in the build log, as these can be a source of test flakiness.
 unstubbed_regex = /   Inside Test: -\[(\w+) (\w+)/m
 if test_log.match(unstubbed_regex)
   output = "#### Found unstubbbed networking requests\n"
@@ -69,7 +79,7 @@ if test_log.match(unstubbed_regex)
   warn(output)
 end
 
-# look at the top 1000 symbols
+# We want to understand how much Swift is slowing our compilation cycles by, so look at the top 1000 Swift symbols from the build log 
 most_expensive_swift_table = `cat #{build_file} | egrep '\.[0-9]ms' | sort -t "." -k 1 -n | tail -1000 | sort -t "." -k 1 -n -r`
 
 # each line looks like "29.2ms  /Users/distiller/eigen/Artsy/View_Controllers/Live_Auctions/LiveAuctionLotViewController.swift:50:19    @objc override func viewDidLoad()"
@@ -79,6 +89,7 @@ time_values = most_expensive_swift_table.lines.map { |line| line.split.first.to_
 require_relative "config/enumerable_stats"
 outliers = time_values.outliers(3)
 
+# Take any build timeing outliers, and convert them into a useful markdown table.  
 if outliers.any?
   warn("Detected some Swift building time outliers")
 
@@ -95,6 +106,7 @@ if outliers.any?
   markdown(([headings] + warnings).join)
 end
 
+# Use the `danger-swiftlint` plugin to lint our modified, and new files.
 swiftlint.config_file = 'Artsy/.swiftlint.yml'
 swiftlint.lint_files
 

Gemfile.lock

@@ -44,6 +44,10 @@ GEM
     celluloid-supervision (0.20.5)
       timers (>= 4.1.1)
     claide (1.0.0)
+    claide-plugins (0.9.0)
+      cork (~> 0)
+      nap (~> 1.0)
+      open4 (~> 1.3)
     cocoapods (1.0.0)
       activesupport (>= 4.0.2)
       claide (>= 1.0.0, < 2.0)
@@ -83,11 +87,13 @@ GEM
     colored (1.2)
     cork (0.1.0)
       colored (~> 1.2)
-    danger (0.8.5)
+    danger (0.10.0)
       claide (~> 1.0)
+      claide-plugins (~> 0.9)
       colored (~> 1.2)
       cork (~> 0.1)
       faraday (~> 0)
+      faraday-http-cache (~> 1.0)
       git (~> 1)
       octokit (~> 4.2)
       redcarpet (~> 3.3)
@@ -96,6 +102,8 @@ GEM
     escape (0.0.4)
     faraday (0.9.2)
       multipart-post (>= 1.2, < 3)
+    faraday-http-cache (1.3.0)
+      faraday (~> 0.8)
     fourflusher (0.3.0)
     fuzzy_match (2.0.4)
     git (1.3.0)
@@ -118,6 +126,7 @@ GEM
       mini_portile2 (~> 2.0.0.rc2)
     octokit (4.3.0)
       sawyer (~> 0.7.0, >= 0.5.3)
+    open4 (1.3.4)
     osx_keychain (1.0.1)
       RubyInline (~> 3)
     redcarpet (3.3.4)