[#493] Integrate Danger to the generated project #579
Conversation
thawzintoe-ptut
temporarily deployed
to
template-compose
GitHub Actions
Inactive
Kover report for template-compose:🧛 Template - Compose Unit Tests Code Coverage:
|
| File | Coverage |
|---|
Modified Files Not Found In Coverage Report:
Dangerfile
Gemfile
Gemfile
Gemfile.lock
review_pull_request.yml
template_review_pull_request.yml
Codebase cunningly covered by count Shroud 🧛
Generated by 🚫 Danger
thawzintoe-ptut
requested review from
AVI5HEK,
Johnsonmaung,
chornerman,
doannimble,
hoangnguyen92dn,
kaungkhantsoe,
luongvo,
manh-t,
minhnimble,
ryan-conway,
sleepylee,
thiennguyen0196 and
toby-thanathip
as code owners
thawzintoe-ptut
had a problem deploying
to
template-compose
GitHub Actions
Error
thawzintoe-ptut
temporarily deployed
to
template-compose
GitHub Actions
Inactive
thawzintoe-ptut
temporarily deployed
to
template-compose
GitHub Actions
Inactive
thawzintoe-ptut
temporarily deployed
to
template-compose
GitHub Actions
Inactive
thawzintoe-ptut
force-pushed
the
feature/493-integrate-danger-to-the-generated-project
branch
from
8c6984f to
db8238c
Compare
thawzintoe-ptut
had a problem deploying
to
template-compose
GitHub Actions
Error
thawzintoe-ptut
force-pushed
the
feature/493-integrate-danger-to-the-generated-project
branch
from
db8238c to
003bb3b
Compare
thawzintoe-ptut
temporarily deployed
to
template-compose
GitHub Actions
Inactive
thawzintoe-ptut
temporarily deployed
to
template-compose
GitHub Actions
Inactive
thawzintoe-ptut
force-pushed
the
feature/493-integrate-danger-to-the-generated-project
branch
from
bda55a8 to
b6a4d6b
Compare
thawzintoe-ptut
temporarily deployed
to
template-compose
GitHub Actions
Inactive
ryan-conway
requested review from
Copilot
and removed request for
doannimble and
thiennguyen0196
There was a problem hiding this comment.
Pull Request Overview
Integrates Danger into the Android template project to automate pull request quality checks via GitHub Actions.
- Adds Gemfiles to manage Danger and related plugins
- Introduces a Dangerfile with rules for PR size, description, labels, linting, and test coverage
- Updates GitHub workflows to set up Ruby, cache dependencies, and run Danger
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| template-compose/Gemfile | New Gemfile listing Danger plugins |
| template-compose/Dangerfile | Danger rules for lint, commit checks, and coverage |
| template-compose/.github/workflows/template_review_pull_request.yml | Workflow to install Ruby and run Danger |
| Gemfile | Root Gemfile updated for CI gems |
| .github/workflows/review_pull_request.yml | Streamlined Ruby setup with bundler-cache for Danger |
Comments suppressed due to low confidence (1)
template-compose/Dangerfile:37
- The placeholder
$projectmay not interpolate in Danger; use Ruby string interpolation (e.g.,"## Kover report for #{project_name}:") or replace with a concrete value.
markdown "## Kover report for $project:"
| @@ -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" | |||
There was a problem hiding this comment.
Typo in user-visible warning: replace 'classed' with 'classified' to correct the message.
| warn("PR is classed as Work in Progress") if github.pr_title.include? "WIP" | |
| warn("PR is classified as Work in Progress") if github.pr_title.include? "WIP" |
|
|
||
| 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 |
There was a problem hiding this comment.
On newer version, we need to clarify the param name, for your reference: https://github.com/Jollibee-Foods-Corporation/jfc-global-templates-android/pull/1326#discussion_r2348637787
| - name: Set up Ruby | ||
| uses: ruby/setup-ruby@v1 | ||
| with: | ||
| ruby-version: '2.7' |
There was a problem hiding this comment.
We should take this chance to update ruby-version to the latest version. I think 3.3.0 could work as we already verified on our current projects
There was a problem hiding this comment.
Shouldn't this be review_pull_request? 🤔
…d remove future enhancement comments
thawzintoe-ptut
force-pushed
the
feature/493-integrate-danger-to-the-generated-project
branch
from
b6a4d6b to
32fc356
Compare
thawzintoe-ptut
had a problem deploying
to
template-compose
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
template-compose/.github/workflows/template_review_pull_request.yml (1)
16-16: Verify environment configuration consistency.The workflow uses
environment: staging, which may differ from the main workflow's environment configuration. Ensure this aligns with your repository's environment setup for proper secrets access.Run the following script to check environment references across workflows:
#!/bin/bash # Check environment configurations across all workflows rg -n "environment:\s*\w+" .github/ template-compose/.github/ .cicdtemplate/.github/template-compose/Dangerfile (2)
2-2: Fix typo in warning message.The word "classed" should be "classified" for correct grammar in the user-facing warning message.
Apply this diff:
-warn("PR is classed as Work in Progress") if github.pr_title.include? "WIP" +warn("PR is classified as Work in Progress") if github.pr_title.include? "WIP"
36-38: Fix undefined variable and verify parameter names.The
$projectvariable is undefined, which will result in incorrect report titles. Additionally, theshroud.reportKovermethod signature may require explicit parameter names in newer versions.Apply this diff:
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 +markdown "## Kover report for template-compose:" +shroud.reportKover( + report_title: "template-compose: Unit Tests", + report_file_path: kover_file_template, + minimum_project_coverage_percentage: 80, + minimum_updated_file_coverage_percentage: 95, + fail_if_under: false +)Based on learnings, newer versions of danger-shroud require explicit parameter names.
🧹 Nitpick comments (2)
template-compose/.github/workflows/template_review_pull_request.yml (1)
24-25: Update actions/cache to latest version.The workflow uses
actions/cache@v2, while.cicdtemplate/.github/workflows/review_pull_request.ymlusesactions/cache@v4. Using the latest version ensures access to performance improvements and bug fixes.Apply this diff:
- name: Cache Gradle - uses: actions/cache@v2 + uses: actions/cache@v4 with:.cicdtemplate/Gemfile (1)
7-16: Pin gem versions for reproducible CI/CD builds.The Gemfile currently declares all Danger gems without version constraints, which can lead to non-reproducible builds and potential breaking changes when new versions are released. This is especially important for CI/CD tooling.
Consider pinning versions for better stability:
# Core Danger functionality -gem 'danger' +gem 'danger', '~> 9.5' # Code quality plugins -gem 'danger-android_lint' # Android Lint integration -gem 'danger-commit_lint' # Commit message validation -gem 'danger-kotlin_detekt' # Detekt static analysis integration +gem 'danger-android_lint', '~> 0.0' # Android Lint integration +gem 'danger-commit_lint', '~> 0.0' # Commit message validation +gem 'danger-kotlin_detekt', '~> 0.0' # Detekt static analysis integration # Test coverage plugin -gem 'danger-shroud' # Kover test coverage reporting +gem 'danger-shroud', '~> 2.0' # Kover test coverage reporting
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
template-compose/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.cicdtemplate/.github/workflows/review_pull_request.yml(1 hunks).cicdtemplate/Dangerfile(1 hunks).cicdtemplate/Gemfile(1 hunks).github/workflows/review_pull_request.yml(2 hunks)Gemfile(0 hunks)template-compose/.github/workflows/template_review_pull_request.yml(1 hunks)template-compose/Dangerfile(1 hunks)template-compose/Gemfile(1 hunks)
💤 Files with no reviewable changes (1)
- Gemfile
🚧 Files skipped from review as they are similar to previous changes (2)
- template-compose/Gemfile
- .github/workflows/review_pull_request.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Review pull request
- GitHub Check: Verify newproject script
- GitHub Check: Run Detekt and unit tests
🔇 Additional comments (2)
.cicdtemplate/.github/workflows/review_pull_request.yml (1)
46-50: LGTM! Good modernization of Ruby setup.The upgrade to Ruby 3.4 with
bundler-cache: truesimplifies the workflow by automating gem caching and installation, removing the need for manual cache and bundle install steps..cicdtemplate/Dangerfile (1)
24-65: Excellent implementation with defensive checks.The Dangerfile demonstrates good practices:
- Section headers improve readability
- Checks for report existence before processing
- Provides helpful guidance messages when reports are missing
- Uses explicit parameter names for
shroud.reportKover, which is compatible with newer versions of the pluginThis is a more robust implementation compared to the template-compose version.
| @@ -0,0 +1,65 @@ | |||
| # 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" | |||
There was a problem hiding this comment.
Fix typo in warning message.
The word "classed" should be "classified" for correct grammar in the user-facing warning message.
Apply this diff:
-warn("PR is classed as Work in Progress") if github.pr_title.include? "WIP"
+warn("PR is classified as Work in Progress") if github.pr_title.include? "WIP"📝 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.
| warn("PR is classed as Work in Progress") if github.pr_title.include? "WIP" | |
| warn("PR is classified as Work in Progress") if github.pr_title.include? "WIP" |
🤖 Prompt for AI Agents
In .cicdtemplate/Dangerfile around line 2, the user-facing warning message
contains a typo: change the string "PR is classed as Work in Progress" to "PR is
classified as Work in Progress" and keep the rest of the line unchanged so the
condition (if github.pr_title.include? "WIP") still triggers the warning.
| - name: Set up Ruby | ||
| uses: ruby/setup-ruby@v1 | ||
| with: | ||
| ruby-version: '2.7' | ||
| bundler-cache: true # runs 'bundle install' and caches installed gems automatically |
There was a problem hiding this comment.
Align Ruby version with other workflows.
This workflow uses Ruby 2.7, while .cicdtemplate/.github/workflows/review_pull_request.yml was updated to Ruby 3.4. This inconsistency could lead to different behavior across templates.
Apply this diff to align with the updated Ruby version:
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
- ruby-version: '2.7'
+ ruby-version: '3.4'
bundler-cache: true # runs 'bundle install' and caches installed gems automatically📝 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.
| - name: Set up Ruby | |
| uses: ruby/setup-ruby@v1 | |
| with: | |
| ruby-version: '2.7' | |
| bundler-cache: true # runs 'bundle install' and caches installed gems automatically | |
| - name: Set up Ruby | |
| uses: ruby/setup-ruby@v1 | |
| with: | |
| ruby-version: '3.4' | |
| bundler-cache: true # runs 'bundle install' and caches installed gems automatically |
🤖 Prompt for AI Agents
In template-compose/.github/workflows/template_review_pull_request.yml around
lines 41 to 45, the workflow pins ruby-version to '2.7' which is inconsistent
with the updated template that uses Ruby 3.4; update the ruby-version value to
'3.4' so this workflow aligns with
.cicdtemplate/.github/workflows/review_pull_request.yml, leaving the
bundler-cache setting unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.cicdtemplate/Dangerfile (1)
2-2: Fix typo in warning message (duplicate of previous review).As noted in a previous review, "classed" should be "classified" for correct grammar.
Apply this diff:
-warn("PR is classed as Work in Progress") if github.pr_title.include? "WIP" +warn("PR is classified as Work in Progress") if github.pr_title.include? "WIP"
🧹 Nitpick comments (1)
.cicdtemplate/Dangerfile (1)
41-52: LGTM! Consistent defensive pattern for Lint checks.The Android Lint section follows the same excellent defensive pattern as Detekt, ensuring reports exist before processing and providing actionable warnings.
Optional: The glob pattern
"**/**/build/reports/lint/lint-result.xml"has redundant wildcards. Consider simplifying to"**/build/reports/lint/lint-result.xml"for consistency with the Detekt pattern.-lint_dir = "**/**/build/reports/lint/lint-result.xml" +lint_dir = "**/build/reports/lint/lint-result.xml"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.cicdtemplate/Dangerfile(1 hunks)Dangerfile(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Review pull request
- GitHub Check: Verify newproject script
- GitHub Check: Run Detekt and unit tests
🔇 Additional comments (4)
Dangerfile (1)
37-42: LGTM! Clear refactor to keyword arguments.The refactor from positional to keyword arguments improves code clarity and makes the intent of each parameter explicit. The functionality remains unchanged.
.cicdtemplate/Dangerfile (3)
13-18: LGTM! Commit quality checks configured appropriately.The commit lint configuration with
warn: :allprovides helpful feedback without blocking PRs, and disablingsubject_lengthis a reasonable choice for flexibility.
24-35: LGTM! Excellent defensive coding pattern.The pattern of checking for report existence before processing and providing a helpful warning if missing is a significant improvement over unconditionally iterating. This provides better developer experience.
54-70: LGTM! Well-structured Kover coverage checks.The Kover section follows the same defensive pattern with file existence checks and helpful warnings. The use of keyword arguments for
shroud.reportKoverimproves clarity, and the threshold configuration is well-documented.
thawzintoe-ptut
had a problem deploying
to
template-compose
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.cicdtemplate/.github/actions/setup-ruby/action.yml (1)
1-26:
This action is identical to the one intemplate-compose/.github/actions/setup-ruby/action.yml. See the review comment on that file regarding the triplication of this action across the codebase..github/actions/setup-ruby/action.yml (1)
1-26:
This is the third identical copy of the Ruby setup action. See the review comment ontemplate-compose/.github/actions/setup-ruby/action.ymlregarding the recommendation to consolidate these three copies into a single shared action.
🧹 Nitpick comments (2)
template-compose/.github/actions/setup-ruby/action.yml (1)
1-26: Action is well-designed but duplicated across three directories.The composite action is properly structured with clear inputs and documentation. However, this action is defined identically in three locations:
template-compose/.github/actions/setup-ruby/action.yml,.cicdtemplate/.github/actions/setup-ruby/action.yml, and.github/actions/setup-ruby/action.yml.Consider whether all three copies are necessary (e.g., due to template generation requirements) or if a single shared action could be referenced instead to reduce maintenance burden.
template-compose/.github/workflows/template_review_pull_request.yml (1)
24-25: Upgrade cache action to v4 for consistency.Line 25 uses
actions/cache@v2, but.cicdtemplate/.github/workflows/review_pull_request.ymlusesv4. For consistency and to benefit from improvements in the newer version, upgrade to@v4.- name: Cache Gradle - uses: actions/cache@v2 + uses: actions/cache@v4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.cicdtemplate/.github/actions/setup-ruby/action.yml(1 hunks).cicdtemplate/.github/workflows/review_pull_request.yml(1 hunks).github/actions/setup-ruby/action.yml(1 hunks).github/workflows/review_pull_request.yml(2 hunks)template-compose/.github/actions/setup-ruby/action.yml(1 hunks)template-compose/.github/workflows/template_review_pull_request.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/review_pull_request.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Verify newproject script
- GitHub Check: Review pull request
- GitHub Check: Run Detekt and unit tests
🔇 Additional comments (3)
.cicdtemplate/.github/workflows/review_pull_request.yml (1)
46-49: Good consolidation of Ruby setup and bundler caching.Moving to a local composite action reduces duplication and simplifies the workflow. The setup correctly enables bundler-cache within the local action.
template-compose/.github/workflows/template_review_pull_request.yml (2)
11-22: Verify environment configuration for secrets access.Line 16 specifies
environment: staging, but based on past review feedback, this may not align with the intended GitHub environment for PR validation. If this workflow requires access toGITHUB_TOKENor other secrets, verify that thestagingenvironment is properly configured with those secrets.Consider whether this should be
environment: template-composeto match the authorization scope intended for this template's PR reviews.
41-50: Ruby setup and Danger integration look correct.The workflow properly uses the local composite action and specifies Ruby 3.3, aligning with the
.cicdtemplatetemplate. Danger configuration withGITHUB_TOKENis appropriate for PR annotations.
#493
What happened 👀
CI/CD pipeline specifically for reviewing pull requests within our Android project template. A total of 76 additions have been made to the
.github/workflows/repiew_pull_request.ymlfile, enhancing its capabilities and adding new steps for better code quality checks and environment management.Insight 📝
Proof Of Work 📹
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.