Extract danger reporting infrastructure into reusable workflows and gem #15

numbata · 2025-12-17T00:16:08Z

Moves reporting responsibility from consuming gems into ruby-grape-danger, establishing it as the authoritative Danger integration framework for Grape projects.

Key changes:

Add RubyGrapeDanger::Reporter class for consistent danger report generation
Implement automatic reporting via at_exit hook in gem's Dangerfile
Add reusable GitHub Actions workflows (danger-run.yml, danger-comment.yml) with embedded script for posting/updating PR comments
Consuming gems now just import ruby-grape-danger and add their checks

Benefits:

DRY: Workflows defined once, reused by all projects
Consistent: All projects use same reporting format and behavior
Maintainable: Fix bugs in workflows once, all projects benefit
Scalable: New danger checks automatically get reporting infrastructure
Simple API: Projects only need danger.import_dangerfile(gem: 'ruby-grape-danger')

Example usage:

  danger.import_dangerfile(gem: 'ruby-grape-danger')
  changelog.check!
  # Reporting happens automatically via at_exit hook

Includes comprehensive specs for Reporter class with 13 test cases covering:

JSON report generation and formatting
Message type handling (strings, objects with .message method)
Edge cases (missing files, missing PR number, empty arrays, nil values)
Multiline content preservation

Moves reporting responsibility from consuming gems into ruby-grape-danger, establishing it as the authoritative Danger integration framework for Grape projects. Key changes: - Add RubyGrapeDanger::Reporter class for consistent danger report generation - Implement automatic reporting via at_exit hook in gem's Dangerfile - Add reusable GitHub Actions workflows (danger-run.yml, danger-comment.yml) with embedded script for posting/updating PR comments - Consuming gems now just import ruby-grape-danger and add their checks Benefits: ✅ DRY: Workflows defined once, reused by all projects ✅ Consistent: All projects use same reporting format and behavior ✅ Maintainable: Fix bugs in workflows once, all projects benefit ✅ Scalable: New danger checks automatically get reporting infrastructure ✅ Simple API: Projects only need `danger.import_dangerfile(gem: 'ruby-grape-danger')` Example usage: danger.import_dangerfile(gem: 'ruby-grape-danger') changelog.check! # Reporting happens automatically via at_exit hook Includes comprehensive specs for Reporter class with 13 test cases covering: - JSON report generation and formatting - Message type handling (strings, objects with .message method) - Edge cases (missing files, missing PR number, empty arrays, nil values) - Multiline content preservation

Just capture status_report as a local variable before at_exit block. Ruby closures allow the at_exit block to access the outer scope variable. Much simpler than module variables or separate classes.

This is the most reliable way since status_report is a method that returns the mutable report object populated by danger checks.

These workflows provide the shared Danger integration infrastructure for Grape projects. The old single danger.yml is replaced by two-stage workflow pattern: - danger-run.yml: Execute danger checks and export report - danger-comment.yml: Post/update PR comment with results

- Add subsections to Table of Contents for Reusable Workflows section - Promote Table of Contents to proper heading level (#) - Add placeholder entry in CHANGELOG for next contribution

dblock

👏

dblock · 2025-12-17T13:59:04Z

@numbata I'm adding you as maintainer here as well

Copilot

Pull request overview

This PR extracts Danger reporting infrastructure into reusable GitHub Actions workflows and establishes ruby-grape-danger as a centralized framework for Danger integration across Grape projects.

Key Changes:

Introduces RubyGrapeDanger::Reporter class for generating JSON reports from Danger status
Implements automatic report export via at_exit hook in the gem's Dangerfile
Adds reusable GitHub Actions workflows (danger-run.yml, danger-comment.yml) with embedded commenting script

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`lib/ruby-grape-danger/reporter.rb`	New Reporter class that generates JSON reports from Danger status with message normalization
`lib/ruby-grape-danger.rb`	Module definition and reporter require statement
`spec/ruby-grape-danger/reporter_spec.rb`	Comprehensive test suite with 13 test cases covering Reporter functionality and edge cases
`Dangerfile`	Adds at_exit hook to automatically export reports using Reporter class via ObjectSpace
`.github/workflows/danger-run.yml`	New reusable workflow that executes Danger checks and uploads report artifact
`.github/workflows/danger-comment.yml`	New reusable workflow with embedded script to post/update PR comments from report
`.github/workflows/danger.yml`	Deleted old workflow in favor of new reusable workflow pattern
`README.md`	Updated documentation explaining reusable workflows architecture, benefits, and usage examples
`CHANGELOG.md`	Added entry documenting this infrastructure change

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-17T19:59:47Z

Dangerfile

+# Register at_exit hook to export report when Dangerfile finishes
+at_exit do
+  # Only skip if there's an actual exception (not SystemExit from danger calling exit)
+  next if $ERROR_INFO && !$ERROR_INFO.is_a?(SystemExit)
+
+  # Find the Dangerfile instance and get its current status_report
+  ObjectSpace.each_object(Danger::Dangerfile) do |df|
+    reporter = RubyGrapeDanger::Reporter.new(df.status_report)
+    reporter.export_json(
+      ENV.fetch('DANGER_REPORT_PATH', nil),
+      ENV.fetch('GITHUB_EVENT_PATH', nil)
+    )
+    break
+  end


Using ObjectSpace.each_object to find the Dangerfile instance is fragile and could be problematic if multiple Dangerfile instances exist in memory. Consider storing a reference to the Dangerfile instance in a way that doesn't require ObjectSpace traversal, or document the assumption that only one Dangerfile instance exists during execution.

Suggested change

# Register at_exit hook to export report when Dangerfile finishes

at_exit do

# Only skip if there's an actual exception (not SystemExit from danger calling exit)

next if $ERROR_INFO && !$ERROR_INFO.is_a?(SystemExit)

# Find the Dangerfile instance and get its current status_report

ObjectSpace.each_object(Danger::Dangerfile) do |df|

reporter = RubyGrapeDanger::Reporter.new(df.status_report)

reporter.export_json(

ENV.fetch('DANGER_REPORT_PATH', nil),

ENV.fetch('GITHUB_EVENT_PATH', nil)

)

break

end

# Capture the Dangerfile instance evaluating this file so we don't need ObjectSpace traversal.

dangerfile_instance = self if defined?(Danger::Dangerfile) && is_a?(Danger::Dangerfile)

# Register at_exit hook to export report when Dangerfile finishes

at_exit do

# Only skip if there's an actual exception (not SystemExit from danger calling exit)

next if $ERROR_INFO && !$ERROR_INFO.is_a?(SystemExit)

# Use the captured Dangerfile instance (if any) to get its current status_report

df = dangerfile_instance

next unless df

reporter = RubyGrapeDanger::Reporter.new(df.status_report)

reporter.export_json(

ENV.fetch('DANGER_REPORT_PATH', nil),

ENV.fetch('GITHUB_EVENT_PATH', nil)

)

Copilot · 2025-12-17T19:59:48Z

.github/workflows/danger-run.yml

+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3


Inconsistent action version: The workflow uses actions/checkout@v3 while danger-comment.yml uses actions/checkout@v4. For consistency and to benefit from the latest improvements and security fixes, both workflows should use the same (preferably latest) version.

Suggested change

uses: actions/checkout@v3

uses: actions/checkout@v4

Copilot · 2025-12-17T19:59:48Z

lib/ruby-grape-danger/reporter.rb

+    def export_json(report_path, event_path)
+      return unless report_path && event_path && File.exist?(event_path)
+
+      event = JSON.parse(File.read(event_path))


The JSON parsing lacks error handling. If the event file contains invalid JSON, this will raise a JSON::ParserError that propagates up. Consider wrapping this in a rescue block to handle malformed JSON gracefully, similar to how the code handles missing files or missing PR numbers.

Suggested change

event = JSON.parse(File.read(event_path))

begin

event = JSON.parse(File.read(event_path))

rescue JSON::ParserError

return

end

Copilot · 2025-12-17T19:59:48Z

.github/workflows/danger-comment.yml

+            }
+
+            // Fail if there are errors
+            if (report.errors && report.errors.length > 0) {


The error checking is inconsistent. This checks for both existence and length of the errors array, but the hasItems function already performs both checks. Consider using hasItems(report.errors) here for consistency with the rest of the script.

Suggested change

if (report.errors && report.errors.length > 0) {

if (hasItems(report.errors)) {

Copilot · 2025-12-17T19:59:49Z

lib/ruby-grape-danger/reporter.rb

+      Array(items).map do |item|
+        item.respond_to?(:message) ? item.message : item.to_s


The to_messages method calls to_s on items that don't respond to :message. This means nil values will become the string "nil" rather than being filtered out. Consider whether empty or nil items should be excluded from the output, or if the current behavior is intentional for debugging purposes.

Suggested change

Array(items).map do |item|

item.respond_to?(:message) ? item.message : item.to_s

Array(items).each_with_object([]) do |item, messages|

msg = item.respond_to?(:message) ? item.message : item

next if msg.nil?

str = msg.to_s

next if str.empty?

messages << str

Copilot · 2025-12-17T19:59:49Z

spec/ruby-grape-danger/reporter_spec.rb

+      it 'does not create a file' do
+        reporter.export_json(nil, event_file.path)
+
+        # If file was created, we would have a different path
+        expect(File.exist?(report_file.path)).to be true


This test doesn't correctly validate the intended behavior. When report_path is nil, the export_json method returns early and doesn't create any file. However, this test is checking if report_file.path exists, which is a different file created in the let block. The test should verify that no report was written, not that an unrelated file exists. The test description and assertion are contradictory.

Suggested change

it 'does not create a file' do

reporter.export_json(nil, event_file.path)

# If file was created, we would have a different path

expect(File.exist?(report_file.path)).to be true

it 'does not write a report file' do

reporter.export_json(nil, event_file.path)

expect(File.size(report_file.path)).to eq(0)

Copilot · 2025-12-17T19:59:49Z

.github/workflows/danger-run.yml

+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 2.7


The Ruby version 2.7 has reached end-of-life (EOL) in March 2023 and is no longer receiving security updates. Consider using a more recent Ruby version like 3.0 or later for better security and performance. This is especially important for a reusable workflow that will be used across multiple projects.

Suggested change

ruby-version: 2.7

ruby-version: '3.1'

Copilot · 2025-12-17T19:59:50Z

lib/ruby-grape-danger/reporter.rb

+  class Reporter
+    def initialize(status_report)
+      @status_report = status_report
+    end


The Reporter class and its public methods lack documentation. Consider adding class-level and method-level documentation to describe the purpose, parameters, and return values. This is particularly important since this is infrastructure code that other projects will rely on.

Copilot · 2025-12-17T19:59:50Z

README.md

 [![Build Status](https://travis-ci.org/ruby-grape/danger.svg?branch=master)](https://travis-ci.org/ruby-grape/danger)

-## Table of Contents
+# Table of Contents


The heading formatting is inconsistent. The README uses '# Table of Contents' (single hash) for this section, while other sections like '## Setup' use double hashes. For consistency within the document structure, this should be '## Table of Contents'.

Suggested change

# Table of Contents

## Table of Contents

Copilot · 2025-12-17T19:59:50Z

README.md

+    uses: ruby-grape/ruby-grape-danger/.github/workflows/danger-run.yml@main
+```
+
+Create `.github/workflows/danger-comment.yml`:
+
+```yaml
+name: Danger Comment
+on:
+  workflow_run:
+    workflows: [Danger]
+    types: [completed]
+  workflow_call:
+
+jobs:
+  comment:
+    uses: ruby-grape/ruby-grape-danger/.github/workflows/danger-comment.yml@main


The README examples reference the reusable Danger workflows using the mutable @main branch (in the uses: ruby-grape/ruby-grape-danger/.github/workflows/...@main lines). If downstream projects copy this configuration, a compromise of the ruby-grape-danger repository (or a malicious change to main) would immediately run attacker-controlled workflow code in their CI with access to their GITHUB_TOKEN and build artifacts. To avoid this supply-chain risk, update the examples to pin the reusable workflows to a specific commit SHA so consumers are not bound to a mutable branch reference.

numbata added 10 commits December 17, 2025 01:12

Update CHANGELOG.md

c1ebf15

Document how to import ruby-grape-danger Dangerfile for reporting

fe0acfc

Add debug logging to at_exit hook

0b458cc

Add comprehensive debugging to at_exit hook

e89d519

Simplify at_exit hook - try direct status_report access

5c00f2b

Simplify report capture - use closure to access status_report

aa3026e

Just capture status_report as a local variable before at_exit block. Ruby closures allow the at_exit block to access the outer scope variable. Much simpler than module variables or separate classes.

Use ObjectSpace to access Dangerfile status_report in at_exit

7ca183d

This is the most reliable way since status_report is a method that returns the mutable report object populated by danger checks.

Update documentation for reusable workflows

6c9ab1b

- Add subsections to Table of Contents for Reusable Workflows section - Promote Table of Contents to proper heading level (#) - Add placeholder entry in CHANGELOG for next contribution

numbata mentioned this pull request Dec 17, 2025

Migrate to reusable danger workflows from ruby-grape-danger dblock/danger-changelog#69

Merged

dblock approved these changes Dec 17, 2025

View reviewed changes

dblock merged commit b037869 into ruby-grape:master Dec 17, 2025
1 check passed

numbata requested a review from Copilot December 17, 2025 19:52

Copilot started reviewing on behalf of numbata December 17, 2025 19:53 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

	if (report.errors && report.errors.length > 0) {
	if (hasItems(report.errors)) {

		Array(items).map do \|item\|
		item.respond_to?(:message) ? item.message : item.to_s

-      Array(items).map do |item|
-        item.respond_to?(:message) ? item.message : item.to_s
+      Array(items).each_with_object([]) do |item, messages|
+        msg = item.respond_to?(:message) ? item.message : item
+        next if msg.nil?
+        str = msg.to_s
+        next if str.empty?
+        messages << str

Extract danger reporting infrastructure into reusable workflows and gem #15

Extract danger reporting infrastructure into reusable workflows and gem #15

Uh oh!

Conversation

numbata commented Dec 17, 2025

Uh oh!

dblock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dblock commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants