Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#493] Integrate Danger to the generated project #579

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions .github/workflows/review_pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ jobs:
uses: ruby/setup-ruby@v1
with:
ruby-version: '2.7'
bundler-cache: true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove redundant gem caching configuration

The bundler-cache: true in the Ruby setup step already handles gem caching. The separate gem caching step is redundant.

Remove the following redundant cache step:

-      - name: Cache gems
-        uses: actions/cache@v2
-        with:
-          path: vendor/bundle
-          key: ${{ runner.os }}-gems-${{ hashFiles('**/Gemfile.lock') }}
-          restore-keys: |
-            ${{ runner.os }}-gems-

Also applies to: 57-63


- name: Cache gems
uses: actions/cache@v2
Expand All @@ -62,16 +63,6 @@ jobs:
restore-keys: |
${{ runner.os }}-gems-

- name: Install Bundle and check environment versions
run: |
echo 'Install Bundle'
bundle config path vendor/bundle
bundle install
echo 'Check environment setup versions'
ruby --version
gem --version
bundler --version

- name: Run Danger
env:
DANGER_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
name: Review pull request

on:
pull_request:
types: [ opened, reopened, ready_for_review, synchronize ]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
review_pull_request:
if: github.event.pull_request.draft == false
name: Review pull request
runs-on: ubuntu-latest
environment: staging
timeout-minutes: 30
Comment on lines +16 to +17
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Environment configuration needs to be updated

The review comment is correct. There is an inconsistency in environment configuration between the workflows:

  • template-compose/.github/workflows/template_review_pull_request.yml uses environment: staging
  • .github/workflows/review_pull_request.yml uses environment: template-compose

This inconsistency should be resolved by updating the environment in template_review_pull_request.yml to match the main workflow's template-compose environment to ensure proper access to secrets and configurations.

🔗 Analysis chain

Verify environment configuration

The workflow uses environment: staging while the main workflow uses environment: template-compose. This inconsistency might cause issues with secrets access.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for environment references in the repository
rg -l "environment:\s*(staging|template-compose)" .github/

Length of output: 102


Script:

#!/bin/bash
# Check the content of both workflow files to compare environment configurations
echo "=== Content of template_review_pull_request.yml ==="
cat template-compose/.github/workflows/template_review_pull_request.yml
echo -e "\n=== Content of review_pull_request.yml ==="
cat .github/workflows/review_pull_request.yml

Length of output: 3961

steps:
- name: Checkout source code
uses: actions/checkout@v4
with:
fetch-depth: 100

- name: Cache Gradle
uses: actions/cache@v2
with:
path: |
~/.gradle/caches/modules-*
~/.gradle/caches/jars-*
~/.gradle/caches/build-cache-*
key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*') }}
restore-keys: |
${{ runner.os }}-gradle-

- name: Set up JDK 17
uses: actions/setup-java@v3
with:
distribution: 'temurin'
java-version: '17'

- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
ruby-version: '2.7'
bundler-cache: true
Comment on lines +41 to +45
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix YAML syntax error in Ruby setup

There's an indentation error in the Ruby setup step.

-     - name: Set up Ruby
-        uses: ruby/setup-ruby@v1
-        with:
-            ruby-version: '2.7'
-            bundler-cache: true
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: '2.7'
+          bundler-cache: true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
ruby-version: '2.7'
bundler-cache: true
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
ruby-version: '2.7'
bundler-cache: true
🧰 Tools
🪛 yamllint (1.35.1)

[error] 41-41: syntax error: expected , but found ''

(syntax)


- name: Cache gems
uses: actions/cache@v2
with:
path: vendor/bundle
key: ${{ runner.os }}-gems-${{ hashFiles('**/Gemfile.lock') }}
restore-keys: |
${{ runner.os }}-gems-

- name: Install Bundle and check environment versions
run: |
echo 'Install Bundle'
bundle config path vendor/bundle
bundle install
echo 'Check environment setup versions'
ruby --version
gem --version
bundler --version
Comment on lines +55 to +63
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above


- name: Run Danger
env:
DANGER_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: bundle exec danger
38 changes: 38 additions & 0 deletions template-compose/Dangerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# 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("Big PR") if git.lines_of_code > 500

# Warn to encourage a PR description
warn("Please provide a summary in the PR description to make it easier to review") if github.pr_body.length == 0

# Warn to encourage that labels should have been used on the PR
warn("Please add labels to this PR") if github.pr_labels.empty?

# Check commits lint and warn on all checks (instead of failing)
commit_lint.check warn: :all, disable: [:subject_length]

# Detekt output check
detekt_dir = "**/build/reports/detekt/detekt-result.xml"
Dir[detekt_dir].each do |file_name|
kotlin_detekt.skip_gradle_task = true
kotlin_detekt.report_file = file_name
kotlin_detekt.detekt(inline_mode: true)
end

# Android Lint output check
lint_dir = "**/**/build/reports/lint/lint-result.xml"
Dir[lint_dir].each do |file_name|
android_lint.skip_gradle_task = true
android_lint.report_file = file_name
android_lint.lint(inline_mode: true)
end

# Show Danger test coverage report from Kover for templates
# Report coverage of modified files, warn if total project coverage is under 80%
# or if any modified file's coverage is under 95%

kover_file_template = "app/build/reports/kover/report.xml"
markdown "## Kover report for $project:"
shroud.reportKover "$project: Unit Tests", kover_file_template, 80, 95, false
Comment on lines +36 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix template string interpolation in Kover report

The $project variable appears to be undefined. This will likely result in incorrect report titles.

-markdown "## Kover report for $project:"
-shroud.reportKover "$project: Unit Tests", kover_file_template, 80, 95, false
+markdown "## Kover report for template-compose:"
+shroud.reportKover "template-compose: Unit Tests", kover_file_template, 80, 95, false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
kover_file_template = "app/build/reports/kover/report.xml"
markdown "## Kover report for $project:"
shroud.reportKover "$project: Unit Tests", kover_file_template, 80, 95, false
kover_file_template = "app/build/reports/kover/report.xml"
markdown "## Kover report for template-compose:"
shroud.reportKover "template-compose: Unit Tests", kover_file_template, 80, 95, false

11 changes: 11 additions & 0 deletions template-compose/Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

source "https://rubygems.org"

git_source(:github) {|repo_name| "https://github.com/#{repo_name}" }

gem 'danger'
gem 'danger-android_lint'
gem 'danger-commit_lint'
gem 'danger-kotlin_detekt'
gem 'danger-shroud'
117 changes: 117 additions & 0 deletions template-compose/Gemfile.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
GEM
remote: https://rubygems.org/
specs:
addressable (2.8.1)
public_suffix (>= 2.0.2, < 6.0)
ansi (1.5.0)
ast (2.4.2)
claide (1.1.0)
claide-plugins (0.9.2)
cork
nap
open4 (~> 1.3)
colored2 (3.1.2)
cork (0.3.0)
colored2 (~> 3.1)
danger (8.6.1)
claide (~> 1.0)
claide-plugins (>= 0.9.2)
colored2 (~> 3.1)
cork (~> 0.1)
faraday (>= 0.9.0, < 2.0)
faraday-http-cache (~> 2.0)
git (~> 1.7)
kramdown (~> 2.3)
kramdown-parser-gfm (~> 1.0)
no_proxy_fix
octokit (~> 4.7)
terminal-table (>= 1, < 4)
danger-android_lint (0.0.11)
danger-plugin-api (~> 1.0)
oga
danger-commit_lint (0.0.7)
danger-plugin-api (~> 1.0)
danger-kotlin_detekt (0.0.3)
danger-plugin-api (~> 1.0)
danger-plugin-api (1.0.0)
danger (> 2.0)
danger-shroud (0.0.5)
danger-plugin-api (~> 1.0)
nokogiri
faraday (1.10.2)
faraday-em_http (~> 1.0)
faraday-em_synchrony (~> 1.0)
faraday-excon (~> 1.1)
faraday-httpclient (~> 1.0)
faraday-multipart (~> 1.0)
faraday-net_http (~> 1.0)
faraday-net_http_persistent (~> 1.0)
faraday-patron (~> 1.0)
faraday-rack (~> 1.0)
faraday-retry (~> 1.0)
ruby2_keywords (>= 0.0.4)
faraday-em_http (1.0.0)
faraday-em_synchrony (1.0.0)
faraday-excon (1.1.0)
faraday-http-cache (2.4.1)
faraday (>= 0.8)
faraday-httpclient (1.0.1)
faraday-multipart (1.0.4)
multipart-post (~> 2)
faraday-net_http (1.0.1)
faraday-net_http_persistent (1.2.0)
faraday-patron (1.0.0)
faraday-rack (1.0.0)
faraday-retry (1.0.3)
git (1.13.2)
addressable (~> 2.8)
rchardet (~> 1.8)
kramdown (2.4.0)
rexml
kramdown-parser-gfm (1.1.0)
kramdown (~> 2.0)
multipart-post (2.2.3)
nap (1.1.0)
no_proxy_fix (0.1.2)
nokogiri (1.14.3-arm64-darwin)
racc (~> 1.4)
nokogiri (1.14.3-x86_64-darwin)
racc (~> 1.4)
nokogiri (1.14.3-x86_64-linux)
racc (~> 1.4)
octokit (4.25.1)
faraday (>= 1, < 3)
sawyer (~> 0.9)
oga (3.4)
ast
ruby-ll (~> 2.1)
open4 (1.3.4)
public_suffix (5.0.1)
racc (1.6.2)
rchardet (1.8.0)
rexml (3.2.5)
ruby-ll (2.1.2)
ansi
ast
ruby2_keywords (0.0.5)
sawyer (0.9.2)
addressable (>= 2.3.5)
faraday (>= 0.17.3, < 3)
terminal-table (3.0.2)
unicode-display_width (>= 1.1.1, < 3)
unicode-display_width (2.3.0)

PLATFORMS
universal-darwin-21
x86_64-darwin-20
x86_64-linux

DEPENDENCIES
danger
danger-android_lint
danger-commit_lint
danger-kotlin_detekt
danger-shroud

BUNDLED WITH
2.3.1
Loading