From d87aa79f02d79444e2ecb336d5cfcab0a09ed493 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Wed, 21 Aug 2019 15:35:26 -0700 Subject: [PATCH 1/3] Run Danger with GitHub Actions --- .github/workflows/danger.yml | 31 ++++++++++++++ Dangerfile | 83 ------------------------------------ dangerfile.js | 79 ++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 83 deletions(-) create mode 100644 .github/workflows/danger.yml delete mode 100644 Dangerfile create mode 100644 dangerfile.js diff --git a/.github/workflows/danger.yml b/.github/workflows/danger.yml new file mode 100644 index 000000000..ec2312c3e --- /dev/null +++ b/.github/workflows/danger.yml @@ -0,0 +1,31 @@ +name: Danger + +on: [pull_request] + +jobs: + danger: + name: Danger JS + runs-on: ubuntu-latest + steps: + - name: Checkout the Git repository + uses: actions/checkout@v1 + # Danger JS would fail because of the below missing module. + # TODO: Report to Danger JS + - name: Install missing module + run: yarn add -D @babel/plugin-transform-flow-strip-types + # TODO: Figure out why GITHUB_TOKEN isn't enough for Danger JS to create a comment. + # Our dangerfile.js escalates any warnings as failures to get more attention. + # + # Here is the error response from GitHub API: + # + # Request failed [403]: https://api.github.com/repos/TextureGroup/Texture/issues/1635/comments + # Response: { + # "message": "Resource not accessible by integration", + # "documentation_url": "https://developer.github.com/v3/issues/comments/#create-a-comment" + # } + # + # https://github.com/TextureGroup/Texture/pull/1635/checks?check_run_id=200541353 + - name: Danger + uses: danger/danger-js@9.1.8 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/Dangerfile b/Dangerfile deleted file mode 100644 index 147d38906..000000000 --- a/Dangerfile +++ /dev/null @@ -1,83 +0,0 @@ -require 'open-uri' - -source_pattern = /(\.m|\.mm|\.h)$/ - -modified_source_files = git.modified_files.grep(source_pattern) -has_modified_source_files = !modified_source_files.empty? -added_source_files = git.added_files.grep(source_pattern) -has_added_source_files = !added_source_files.empty? - -# Make it more obvious that a PR is a work in progress and shouldn't be merged yet -warn("PR is classed as Work in Progress") if github.pr_title.include? "[WIP]" - -# Warn when there is a big PR -warn("This is a big PR, please consider splitting it up to ease code review.") if git.lines_of_code > 500 - -# Modifying the changelog will probably get overwritten. -if git.modified_files.include?("CHANGELOG.md") && !github.pr_title.include?("#changelog") - warn("PR modifies CHANGELOG.md, which is a generated file. Add #changelog to the title to suppress this warning.") -end - -def full_license(partial_license, filename) - license_header = <<-HEREDOC -// - HEREDOC - license_header += "// " + filename + "\n" - license_header += <<-HEREDOC -// Texture -// - HEREDOC - license_header += partial_license - return license_header -end - -def check_file_header(files_to_check, licenses) - repo_name = github.pr_json["base"]["repo"]["full_name"] - pr_number = github.pr_json["number"] - files = github.api.pull_request_files(repo_name, pr_number) - files.each do |file| - if files_to_check.include?(file["filename"]) - filename = File.basename(file["filename"]) - - data = "" - contents = github.api.get file["contents_url"] - open(contents["download_url"]) { |io| - data += io.read - } - - correct_license = false - licenses.each do |license| - license_header = full_license(license, filename) - if data.include? "Pinterest, Inc." - correct_license = true - end - end - - if correct_license == false - warn ("Please ensure license is correct for #{filename}: \n```\n" + full_license(licenses[0], filename) + "```") - end - - end - end -end - -# Ensure new files have proper header -new_source_license_header = <<-HEREDOC -// Copyright (c) Pinterest, Inc. All rights reserved. -// Licensed under Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0 -HEREDOC - -if has_added_source_files - check_file_header(added_source_files, [new_source_license_header]) -end - -# Ensure modified files have proper header -modified_source_license_header = <<-HEREDOC -// Copyright (c) Facebook, Inc. and its affiliates. All rights reserved. -// Changes after 4/13/2017 are: Copyright (c) Pinterest, Inc. All rights reserved. -// Licensed under Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0 -HEREDOC - -if has_modified_source_files - check_file_header(modified_source_files, [modified_source_license_header, new_source_license_header]) -end diff --git a/dangerfile.js b/dangerfile.js new file mode 100644 index 000000000..29aa5943c --- /dev/null +++ b/dangerfile.js @@ -0,0 +1,79 @@ +const { danger, fail, warn, schedule } = require('danger'); +const { readFileSync } = require('fs'); + +const source_pattern = /(\.m|\.mm|\.h)$/; +const modified_source_files = danger.git.modified_files.filter(f => source_pattern.test(f)); +const has_modified_source_files = (Array.isArray(modified_source_files) && modified_source_files.length > 0); +const added_source_files = danger.git.created_files.filter(f => source_pattern.test(f)); +const has_added_source_files = (Array.isArray(added_source_files) && added_source_files.length > 0); + +// Make it more obvious that a PR is a work in progress and shouldn't be merged yet +if (danger.github.pr.title.includes("[WIP]")) { + fail("PR is classed as Work in Progress"); +} + +// Warn when there is a big PR +if (danger.github.pr.additions + danger.github.pr.deletions > 500) { + warn("This is a big PR, please consider splitting it up to ease code review."); +} + +// Modifying the changelog will probably get overwritten. +if (danger.git.modified_files.includes("CHANGELOG.md")) { + if (danger.github.pr.title.includes("#changelog")) { + warn("PR modifies CHANGELOG.md, which is a generated file. #changelog added to the title to suppress this warning."); + } else { + fail("PR modifies CHANGELOG.md, which is a generated file. Add #changelog to the title to suppress this failure."); + } +} + +// Reference: http://a32.me/2014/03/heredoc-multiline-variable-with-javascript/ +function hereDoc(f) { + return f.toString(). + replace(/^[^\/]+\/\*!?/, ''). + replace(/\*\/[^\/]+$/, ''); +} + +function full_license(partial_license, filename) { + var license_header = hereDoc(function() {/*! +// +*/}); + license_header += "// " + filename; + license_header += hereDoc(function() {/*! +// Texture +//*/}); + license_header += partial_license; + return license_header; +} + +function check_file_header(files_to_check, license) { + for (let file of files_to_check) { + const filename = file.replace(/^.*[\\\/]/, ''); + schedule(async () => { + const content = await danger.github.utils.fileContents(file); + if (!content.includes("Pinterest, Inc.")) { + fail("Please ensure license is correct for " + filename +":\n```" + full_license(license, filename) + "\n```"); + } + }); + } +} + +const new_source_license_header = hereDoc(function() {/*! +// Copyright (c) Pinterest, Inc. All rights reserved. +// Licensed under Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0 +//*/}); + +const modified_source_license_header = hereDoc(function() {/*! +// Copyright (c) Facebook, Inc. and its affiliates. All rights reserved. +// Changes after 4/13/2017 are: Copyright (c) Pinterest, Inc. All rights reserved. +// Licensed under Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0 +//*/}); + +// Ensure new files have proper header +if (has_added_source_files) { + check_file_header(added_source_files, new_source_license_header); +} + +// Ensure modified files have proper header +if (has_modified_source_files) { + check_file_header(modified_source_files, modified_source_license_header); +} From 766c0c24b690d1fa0d2f373ff10a81274eab237a Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Thu, 22 Aug 2019 09:04:24 -0700 Subject: [PATCH 2/3] Print error messages to console --- dangerfile.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/dangerfile.js b/dangerfile.js index 29aa5943c..df3ff85ad 100644 --- a/dangerfile.js +++ b/dangerfile.js @@ -9,20 +9,28 @@ const has_added_source_files = (Array.isArray(added_source_files) && added_sourc // Make it more obvious that a PR is a work in progress and shouldn't be merged yet if (danger.github.pr.title.includes("[WIP]")) { - fail("PR is classed as Work in Progress"); + const msg = "PR is classed as Work in Progress"; + console.error("FAIL: " + msg); + fail(msg); } // Warn when there is a big PR if (danger.github.pr.additions + danger.github.pr.deletions > 500) { - warn("This is a big PR, please consider splitting it up to ease code review."); + const msg = "This is a big PR, please consider splitting it up to ease code review."; + console.error("WARN: "+ msg); + warn(msg); } // Modifying the changelog will probably get overwritten. if (danger.git.modified_files.includes("CHANGELOG.md")) { if (danger.github.pr.title.includes("#changelog")) { - warn("PR modifies CHANGELOG.md, which is a generated file. #changelog added to the title to suppress this warning."); + const msg = "PR modifies CHANGELOG.md, which is a generated file. #changelog added to the title to suppress this warning."; + console.error("WARN: "+ msg); + warn(msg); } else { - fail("PR modifies CHANGELOG.md, which is a generated file. Add #changelog to the title to suppress this failure."); + const msg = "PR modifies CHANGELOG.md, which is a generated file. Add #changelog to the title to suppress this failure."; + console.error("FAIL: " + msg); + fail(msg); } } @@ -51,7 +59,9 @@ function check_file_header(files_to_check, license) { schedule(async () => { const content = await danger.github.utils.fileContents(file); if (!content.includes("Pinterest, Inc.")) { - fail("Please ensure license is correct for " + filename +":\n```" + full_license(license, filename) + "\n```"); + const msg = "Please ensure license is correct for " + filename +":\n```" + full_license(license, filename) + "\n```"; + console.error("FAIL: " + msg); + fail(msg); } }); } From b5ef4e5a5e84c3a24f224116a36c9ae2a3e307c9 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Thu, 22 Aug 2019 10:10:23 -0700 Subject: [PATCH 3/3] Add reference to filename regex --- dangerfile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dangerfile.js b/dangerfile.js index df3ff85ad..b191420c3 100644 --- a/dangerfile.js +++ b/dangerfile.js @@ -55,7 +55,7 @@ function full_license(partial_license, filename) { function check_file_header(files_to_check, license) { for (let file of files_to_check) { - const filename = file.replace(/^.*[\\\/]/, ''); + const filename = file.replace(/^.*[\\\/]/, ''); // Reference: https://stackoverflow.com/a/423385 schedule(async () => { const content = await danger.github.utils.fileContents(file); if (!content.includes("Pinterest, Inc.")) {