chore: integrate Detekt for static code analysis and add configuration files#12
chore: integrate Detekt for static code analysis and add configuration files#12
Conversation
📝 WalkthroughWalkthroughAdds Detekt static analysis and SARIF upload to GitHub Actions, introduces a repository Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Detekt as Detekt
participant Repo as Repository (SARIF)
rect rgba(135,206,235,0.5)
Dev->>GH: push commit / PR
end
rect rgba(144,238,144,0.5)
GH->>GH: run build job (tests, lint)
GH->>Detekt: run Detekt analysis (produces SARIF + HTML)
Detekt-->>GH: output reports (build/reports/detekt/detekt.sarif)
GH->>Repo: upload SARIF via upload-sarif action
Repo-->>GH: Code Scanning receives SARIF
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR integrates Detekt into the Gradle build to provide static analysis (including formatting rules) and uploads Detekt SARIF results to GitHub code scanning. It also updates Kotlin source/test files to comply with the newly enabled Detekt formatting/style rules (e.g., trailing commas, import ordering, removing wildcard imports, and replacing magic numbers with constants).
Changes:
- Add Detekt Gradle plugin + detekt-formatting dependency and introduce a repository-wide
detekt.ymlconfiguration. - Update CI workflow to upload Detekt SARIF results to GitHub Code Scanning.
- Apply formatting/style fixes across Kotlin sources and tests (trailing commas, explicit imports, constants for magic numbers).
Reviewed changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.kt |
Replaces wildcard JUnit import with explicit imports for Detekt compliance. |
src/test/kotlin/dev/twango/jetplay/editor/MediaFileEditorProviderTest.kt |
Applies trailing comma formatting. |
src/main/kotlin/dev/twango/jetplay/transfer/DownloadSession.kt |
Adds a constant to replace a magic number and adjusts formatting. |
src/main/kotlin/dev/twango/jetplay/transcode/TranscodeSession.kt |
Applies trailing comma formatting in constructor params. |
src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt |
Refactors magic numbers into constants and applies formatting/import ordering. |
src/main/kotlin/dev/twango/jetplay/media/MediaClassification.kt |
Re-formats extension set for trailing commas/line wrapping rules. |
src/main/kotlin/dev/twango/jetplay/JetPlayConstants.kt |
Formatting-only change. |
src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.kt |
Formatting-only change. |
src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.kt |
Applies trailing commas and Kotlin Unit-returning expression bodies; formats notification action. |
src/main/kotlin/dev/twango/jetplay/browser/PlayerConfig.kt |
Adds trailing commas in data class declarations. |
gradle/libs.versions.toml |
Adds Detekt plugin + detekt-formatting library coordinates. |
detekt.yml |
Introduces Detekt ruleset configuration (complexity/style/formatting/import ordering). |
build.gradle.kts |
Applies Detekt plugin, wires detekt-formatting, and enables Detekt reports (incl. SARIF). |
.github/workflows/build.yml |
Adds permissions and uploads Detekt SARIF to GitHub code scanning in CI. |
.editorconfig |
Adds editor defaults aligned with Detekt formatting (indentation, line length). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt (1)
22-22: Consider centralizing shared size constants to avoid drift.
BYTES_PER_KBnow exists here and insrc/main/kotlin/dev/twango/jetplay/transfer/DownloadSession.kt(Line 18). A shared constant module would prevent future divergence.Also applies to: 105-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt` at line 22, Centralize the duplicate size constant BYTES_PER_KB by creating a single shared constant (e.g., object or companion object named SizeConstants with BYTES_PER_KB = 1024) and replace the local constants in MediaTranscoder (class MediaTranscoder) and DownloadSession (class DownloadSession) to reference SizeConstants.BYTES_PER_KB; update imports/usages and remove the local BYTES_PER_KB declarations so both classes use the single authoritative constant to prevent future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 145-151: The "Upload Detekt SARIF" step unconditionally runs but
assumes build/reports/detekt/detekt.sarif exists; change its if condition so the
step only runs when that SARIF file is present (e.g., replace always() with a
check using hashFiles for build/reports/detekt/detekt.sarif), keeping the step
name "Upload Detekt SARIF" and the sarif_file setting intact so the upload is
skipped when the report is missing.
In `@gradle/libs.versions.toml`:
- Line 7: The detekt version in libs.versions.toml (the detekt = "1.23.7" entry)
is incompatible with Kotlin 2.3.20; update the detekt dependency string to a
compatible release (e.g., change the detekt entry to "2.0.0-alpha.2") or
alternatively align the Kotlin version down to a supported range (1.8.x–2.0.x)
so that the Detekt tool and Kotlin compiler versions are compatible.
In `@src/main/kotlin/dev/twango/jetplay/media/MediaClassification.kt`:
- Around line 5-15: VIDEO_EXTENSIONS is hardcoded but must be sourced from
plugin.xml; change MediaClassification to load the supported extensions from the
plugin configuration instead of the literal set. Update the companion/object
field that currently defines VIDEO_EXTENSIONS to read the extension list (or a
single config path) from plugin.xml at initialization (e.g., via the plugin
config loader or XML parser your project uses), cache it in a read-only property
(e.g., supportedExtensions or getSupportedExtensions()) and use that property
everywhere VIDEO_EXTENSIONS is referenced so no hardcoded extensions remain in
MediaClassification.
---
Nitpick comments:
In `@src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt`:
- Line 22: Centralize the duplicate size constant BYTES_PER_KB by creating a
single shared constant (e.g., object or companion object named SizeConstants
with BYTES_PER_KB = 1024) and replace the local constants in MediaTranscoder
(class MediaTranscoder) and DownloadSession (class DownloadSession) to reference
SizeConstants.BYTES_PER_KB; update imports/usages and remove the local
BYTES_PER_KB declarations so both classes use the single authoritative constant
to prevent future drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9fb95167-8731-4e35-8f7c-1cab11d2ae82
📒 Files selected for processing (15)
.editorconfig.github/workflows/build.ymlbuild.gradle.ktsdetekt.ymlgradle/libs.versions.tomlsrc/main/kotlin/dev/twango/jetplay/JetPlayConstants.ktsrc/main/kotlin/dev/twango/jetplay/browser/PlayerConfig.ktsrc/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.ktsrc/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.ktsrc/main/kotlin/dev/twango/jetplay/media/MediaClassification.ktsrc/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.ktsrc/main/kotlin/dev/twango/jetplay/transcode/TranscodeSession.ktsrc/main/kotlin/dev/twango/jetplay/transfer/DownloadSession.ktsrc/test/kotlin/dev/twango/jetplay/editor/MediaFileEditorProviderTest.ktsrc/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.kt
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2025.1.1
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt`:
- Around line 25-36: JCEF_NATIVE_EXTENSIONS is hardcoded in MediaTranscoder and
used by needsTranscoding; instead read the supported extensions from plugin
metadata (plugin.xml) via the plugin/component loader and remove the hardcoded
set (also the duplicate at the other constant around line 38). Update
MediaTranscoder to fetch the extension list at startup (or inject a
PluginMetadata/ExtensionRegistry) and have needsTranscoding consult that runtime
list; reference the JCEF_NATIVE_EXTENSIONS constant to remove/replace and the
needsTranscoding method in MediaTranscoder when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bd79a640-995e-45de-a988-796744d4d973
📒 Files selected for processing (13)
build.gradle.ktsdetekt.ymlgradle/libs.versions.tomlsrc/main/kotlin/dev/twango/jetplay/JetPlayConstants.ktsrc/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.ktsrc/main/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoader.ktsrc/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.ktsrc/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.ktsrc/main/kotlin/dev/twango/jetplay/media/MediaClassification.ktsrc/main/kotlin/dev/twango/jetplay/media/RemoteFileMediaSource.ktsrc/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.ktsrc/main/kotlin/dev/twango/jetplay/transcode/TranscodeSession.ktsrc/main/kotlin/dev/twango/jetplay/transfer/DownloadSession.kt
✅ Files skipped from review due to trivial changes (9)
- src/main/kotlin/dev/twango/jetplay/transcode/TranscodeSession.kt
- src/main/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoader.kt
- src/main/kotlin/dev/twango/jetplay/media/MediaClassification.kt
- src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.kt
- src/main/kotlin/dev/twango/jetplay/media/RemoteFileMediaSource.kt
- src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.kt
- build.gradle.kts
- detekt.yml
- gradle/libs.versions.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/kotlin/dev/twango/jetplay/JetPlayConstants.kt
- src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.kt
- src/main/kotlin/dev/twango/jetplay/transfer/DownloadSession.kt
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
Summary by CodeRabbit