Skip to content

Conversation

@raghavsatyadev
Copy link
Contributor

@raghavsatyadev raghavsatyadev commented Nov 5, 2025

  • Configure detekt plugin on core module
  • Run detekt on PR actions like opened, reopened, synchronised
  • baseline for detekt has been setup

Solves #148

- Run detekt on PR actions like opened, reopened, synchronised
- baseline for detekt has been setup
Copilot AI review requested due to automatic review settings November 5, 2025 17:54
Copy link

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 integrates Detekt static code analysis into the project to improve code quality and enforce coding standards. The implementation includes comprehensive configuration for the core module, a baseline to track existing issues, and CI integration.

Key Changes

  • Added Detekt plugin (version 1.23.8) to the project with configuration for the core module
  • Created extensive detekt.yml configuration file with customized rule sets across multiple categories (complexity, style, naming, performance, etc.)
  • Integrated Detekt into the CI/CD pipeline to run on pull requests

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
gradle/libs.versions.toml Added Detekt version 1.23.8 and plugin declaration
detekt.yml Comprehensive Detekt configuration with customized rules for code quality checks
core/detekt-baseline.xml Baseline file tracking 249 pre-existing issues to be addressed incrementally
core/build.gradle.kts Configured Detekt for the core module with source directories, baseline, and reporting options
build.gradle.kts Applied Detekt plugin at the root level
.github/workflows/pr_tests.yml Added Detekt execution step in PR workflow with summary reporting

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

Copy link
Member

@alexstyl alexstyl left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Got a few questions and a few change requests

@raghavsatyadev raghavsatyadev marked this pull request as draft November 6, 2025 12:20
- Simplified the `detekt.yml` configuration by removing many commented-out and default rules, focusing on a stricter, more relevant ruleset.
- Increased `maxIssues` to 10.
- Disabled several complexity and naming rules (e.g., `LongMethod`, `LongParameterList`, `FunctionNaming`) that are less applicable to Jetpack Compose code.
- Updated the Detekt baseline to reflect the new configuration.
- Streamlined the Detekt step in the `pr_tests.yml` GitHub workflow.
@raghavsatyadev raghavsatyadev marked this pull request as ready for review November 6, 2025 12:22
@alexstyl
Copy link
Member

alexstyl commented Nov 6, 2025

Asked for a file to be renamed as the context changed, plus added some comments in the existing discussions.

@raghavsatyadev let me resolve conversations to make sure there is no follow up

Renamed the `pr_tests.yml` workflow file to `ci.yml` for clarity. Also, corrected the Detekt summary output to properly format it as a markdown code block in the workflow log.
Moves the `Run unit tests` step to execute after the Detekt analysis steps to reject the PR early if it has any code smells.
The Detekt summary is now outputted to the GitHub Step Summary for better visibility in the workflow run.
Copy link
Member

@alexstyl alexstyl left a comment

Choose a reason for hiding this comment

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

Asked for the last bits of changes.

Removes most of the custom detekt configuration, opting for detekt's defaults. The maximum issue count is increased to 500 to allow the build to pass, and the baseline file has been cleared.
@alexstyl
Copy link
Member

alexstyl commented Nov 8, 2025

Looks good. Let's merge this.

@alexstyl alexstyl merged commit 978afac into composablehorizons:main Nov 8, 2025
1 check passed
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