Skip to content

Conversation

@JuanVqz
Copy link
Member

@JuanVqz JuanVqz commented Nov 14, 2025

Context

We added support to JSON, and HTML reports.

Done When

We can generate those reports with the --formats flag

skunk . --formats console,json,html # if you need the 3 format reports
skunk app lib --formats json # will only save the json result at `tmp/rubycritic/skunk_report.json`
skunk app # will use console format by default

@JuanVqz JuanVqz force-pushed the feat/cli-multi-out-formats branch from defabea to f6e9562 Compare November 14, 2025 17:37
@JuanVqz JuanVqz changed the title feat(cli): support multiple output formats via --out feat(cli): support multiple output formats via --formats Nov 14, 2025
@JuanVqz JuanVqz changed the title feat(cli): support multiple output formats via --formats feat(cli): support multiple output formats via --formats flag Nov 14, 2025
@JuanVqz JuanVqz force-pushed the feat/cli-multi-out-formats branch 2 times, most recently from 51ce9d3 to 97fca6d Compare November 14, 2025 17:51
Update rubocop todo file with latest offense counts and configurations
@JuanVqz JuanVqz force-pushed the feat/cli-multi-out-formats branch from 97fca6d to 174f70d Compare November 14, 2025 17:56
@JuanVqz JuanVqz requested review from Copilot and fbuys and removed request for Copilot November 14, 2025 17:59
@JuanVqz JuanVqz marked this pull request as ready for review November 14, 2025 17:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for multiple output formats (JSON, HTML, console) through a new --formats CLI flag, allowing users to generate one or more report types in a single run.

Key Changes:

  • Introduced --formats flag to CLI with support for comma-separated format values (json, html, console)
  • Added parsing logic to convert format strings to symbols and store in Skunk::Config
  • Updated documentation to explain CLI and programmatic format configuration

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/skunk/cli/options/argv.rb Adds --formats flag parser that accepts comma-separated formats and stores them in Config
test/lib/skunk/cli/options/argv_test.rb Adds test coverage for the new --formats option parsing
test/lib/skunk/commands/help_test.rb Updates help text to include the new --formats flag
README.md Documents CLI flag usage and programmatic configuration for output formats
CHANGELOG.md Records the new feature addition
.rubocop_todo.yml Updates RuboCop offense counts and removes resolved line length violations

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

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@JuanVqz Changes look good, I just added one comment related to default

self.output_filename = filename
end

opts.on("-f", "--formats json,html,console", Array, "Output formats: json,html,console") do |list|
Copy link
Member

Choose a reason for hiding this comment

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

Any way to tell that the default is console here?

@JuanVqz JuanVqz requested a review from etagwerker November 19, 2025 01:18
Add test case to verify default console format when no --formats option is provided
@JuanVqz JuanVqz force-pushed the feat/cli-multi-out-formats branch from ad47093 to 633f011 Compare November 19, 2025 01:19
Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@JuanVqz Unfortunately now I'm getting non-deterministic failures from the test suite and it seems to be related to these changes.

https://gist.github.com/etagwerker/9ac9d8e20b5f5a4789b5fe263946c211

1 or 2 out of 10 test executions will fail with something like that.

Could you please take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants