From d0ab404c34abe266c035260160b6ea3e61806941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ka=C5=A1par?= Date: Sat, 31 Aug 2019 17:14:17 +0200 Subject: [PATCH] [GitHub CI] Revert back to Ruby Danger as Danger JS is not working properly (#1650) * Update ci.yml Add name to the script run phase * Revert back Dangerfile and remove danger JS, create separate workflow for push for tests and for PR for tests + danger, run ruby danger as part of the PR check * remove PR ci * Revert "remove PR ci" This reverts commit 839ba3ae2a6bc94240735bdcb84075bd7f6f82af. * cahnge token to default one * PR feedback * Rename danger.yml name --- .github/workflows/ci.yml | 3 +- .github/workflows/danger.yml | 31 +++++-------- Dangerfile | 83 +++++++++++++++++++++++++++++++++ build.sh | 7 +++ dangerfile.js | 89 ------------------------------------ 5 files changed, 104 insertions(+), 109 deletions(-) create mode 100644 Dangerfile delete mode 100644 dangerfile.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf9ce035c..61bd5bfce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,4 +29,5 @@ jobs: steps: - name: Checkout the Git repository uses: actions/checkout@v1 - - run: ./build.sh ${{ matrix.mode }} + - name: Run build script + run: ./build.sh ${{ matrix.mode }} diff --git a/.github/workflows/danger.yml b/.github/workflows/danger.yml index 6d2a2e838..c71f053fa 100644 --- a/.github/workflows/danger.yml +++ b/.github/workflows/danger.yml @@ -3,26 +3,19 @@ name: Danger on: [pull_request] jobs: - danger: - name: Danger JS - runs-on: ubuntu-latest + buildsh: + strategy: + matrix: + mode: [danger] + include: + - mode: danger + name: Run Danger + name: ${{ matrix.name }} + runs-on: macOS-10.14 steps: - name: Checkout the Git repository uses: actions/checkout@v1 - # 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 + - name: Run build script + run: ./build.sh ${{ matrix.mode }} env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - DANGER_DISABLE_TRANSPILATION: true + DANGER_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/Dangerfile b/Dangerfile new file mode 100644 index 000000000..147d38906 --- /dev/null +++ b/Dangerfile @@ -0,0 +1,83 @@ +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/build.sh b/build.sh index a58fc11fc..5d8c38575 100755 --- a/build.sh +++ b/build.sh @@ -80,6 +80,13 @@ if [ "$MODE" = "tests" -o "$MODE" = "all" ]; then success="1" fi +if [ "$MODE" = "danger" -o "$MODE" = "all" ]; then + bundle install + echo "Running Danger..." + bundle exec danger --fail-on-errors=true + success="1" +fi + if [ "$MODE" = "tests_listkit" ]; then echo "Building & testing AsyncDisplayKit+IGListKit." pod install --project-directory=SubspecWorkspaces/ASDKListKit diff --git a/dangerfile.js b/dangerfile.js deleted file mode 100644 index b191420c3..000000000 --- a/dangerfile.js +++ /dev/null @@ -1,89 +0,0 @@ -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]")) { - 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) { - 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")) { - 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 { - 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); - } -} - -// 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(/^.*[\\\/]/, ''); // Reference: https://stackoverflow.com/a/423385 - schedule(async () => { - const content = await danger.github.utils.fileContents(file); - if (!content.includes("Pinterest, Inc.")) { - const msg = "Please ensure license is correct for " + filename +":\n```" + full_license(license, filename) + "\n```"; - console.error("FAIL: " + msg); - fail(msg); - } - }); - } -} - -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); -}