Skip to content

Add JSON Report #119

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

Merged
merged 4 commits into from
May 5, 2025
Merged

Add JSON Report #119

merged 4 commits into from
May 5, 2025

Conversation

JuanVqz
Copy link
Member

@JuanVqz JuanVqz commented May 2, 2025

  • Add an entry to CHANGELOG.md that links to this PR under the "main (unreleased)" heading.

Description:
I've been wanting to add the JSON report to the skunk gem since forever, and I hope today is the day.

As you can see in the screenshot, it is still reporting the console Skunk report as it used to be.
Why? because I didn't want to break the current behavior until we first test the json generator and consider it stable (which it should I'm not concerned about it) and two, we support more generators as RubyCritic does, like ⁣html for example.

The report.json file is created under the directory specified in your RubyCritic::Config.root configuration.

Screenshot 2025-05-02 at 44

I will abide by the code of conduct.

@JuanVqz JuanVqz mentioned this pull request May 2, 2025
1 task
Improve bin/console experience
@JuanVqz JuanVqz force-pushed the feature/add-generators branch from 058daf5 to 83bcda3 Compare May 2, 2025 20:50
@JuanVqz JuanVqz force-pushed the feature/add-generators branch from 83bcda3 to 0ee9975 Compare May 2, 2025 21:07
@@ -9,7 +9,7 @@ jobs:
strategy:
matrix:
os: [ubuntu]
ruby-version: ["2.6", "2.7", "3.0", "3.1", "3.2"]
ruby-version: ["2.6", "2.7", "3.0", "3.1", "3.2", "3.3"]
Copy link
Member Author

@JuanVqz JuanVqz May 2, 2025

Choose a reason for hiding this comment

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

Might be out of the scope adding support to Ruby 3.3 but that is the Ruby I had in my machine and the code ran, meaning it seems safe.

@@ -6,11 +6,11 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
Copy link
Member Author

@JuanVqz JuanVqz May 2, 2025

Choose a reason for hiding this comment

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

Bumping GitHub checkout versions

@@ -11,3 +11,4 @@
.byebug_history

.ruby-version
.tool-versions
Copy link
Member Author

Choose a reason for hiding this comment

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

Ignore asdf ruby file

@@ -1,16 +1,14 @@
# frozen_string_literal: true
Copy link
Member Author

@JuanVqz JuanVqz May 2, 2025

Choose a reason for hiding this comment

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

I removed this line because was erroring when using the bin/console command.

IRB.start(__FILE__)
# Run skunk CLI application with the provided arguments
require "skunk/cli/application"
Skunk::Cli::Application.new(ARGV).execute
Copy link
Member Author

Choose a reason for hiding this comment

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

making the bin/console useful, now when it runs you can stop with the debugger everywhere and will run the CLI command with any given arg to improve debugging and timing to develop

cost: file.cost.round(2),
coverage: file.coverage.round(2)
}
end
Copy link
Member Author

Choose a reason for hiding this comment

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

All the private methods are the same in the lib/skunk/commands/status_reporter.rb file, but that file is currently tied to generate the console report, and to not break anything for now, we are duplicating them

@@ -8,7 +8,7 @@
SimpleCov.formatter = SimpleCov::Formatter::MultiFormatter[
SimpleCov::Formatter::HTMLFormatter,
SimpleCov::Formatter::Console,
SimpleCov::Formatter::Codecov,
SimpleCov::Formatter::Codecov
Copy link
Member Author

Choose a reason for hiding this comment

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

Linter wish :)

@JuanVqz JuanVqz force-pushed the feature/add-generators branch from c62a831 to 29d2ab6 Compare May 2, 2025 22:16
@@ -10,6 +10,7 @@ module Command
# reporter object.
class Base < RubyCritic::Command::Base
def initialize(options)
super
Copy link
Member Author

Choose a reason for hiding this comment

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

Making linter happy

@JuanVqz JuanVqz marked this pull request as ready for review May 2, 2025 22:31
@JuanVqz
Copy link
Member Author

JuanVqz commented May 2, 2025

Hey @etagwerker!

I got finally time to finish this feature. In my previous PR I was thinking on adding the json report separated to the current console report and allow the user choose between json and console, however, it would take more time.

This is seemless approach because you will still generating the console report but also generating the json report in background (as the RubyCritic gem does), we can expose the json generator until we consider it stable or add more generators (json/html/console).

How would you like to release this change? It seems safe to just use a regular release, but I was wondering if you want to release a beta or any other way.

@JuanVqz JuanVqz requested a review from etagwerker May 2, 2025 22:37
@JuanVqz JuanVqz changed the title Add JSON generator Add JSON Report May 2, 2025
@@ -27,7 +26,7 @@ def initialize(options)
#
# @return [Skunk::Command::StatusReporter]
def execute
RubyCritic::Config.formats = []
RubyCritic::Config.formats = [:json]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a temporal change, since we currently only support the :json generator; it's defaulted to it. but this will be removed eventually.

@@ -39,6 +38,8 @@ def execute
#
# @param [RubyCritic::AnalysedModulesCollection] A collection of analysed modules
def report(analysed_modules)
Reporter.generate_report(analysed_modules)
Copy link
Member Author

@JuanVqz JuanVqz May 3, 2025

Choose a reason for hiding this comment

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

This was the magic line that allowed us generate the json file in runtime (this is also how RubyCritic works).

In the future, I see Skunk following RubyCritic conventions and extracting the logic out lib/skunk/commands/status_reporter.rb to somewhere else and reusing it in the different generators.

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 This looks great, thank you! 💯

@etagwerker etagwerker merged commit 289a470 into main May 5, 2025
14 checks passed
@etagwerker etagwerker deleted the feature/add-generators branch May 5, 2025 00:30
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.

2 participants