-
Notifications
You must be signed in to change notification settings - Fork 3
Enhance code style and linting #100
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Editor
participant Linter
participant CI
Developer->>Editor: Edits code (editorconfig enforced)
Editor->>Linter: Applies formatting/linting rules
Linter->>CI: Lint and ktlint jobs run first
CI->>CI: On success, triggers build job
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: in the ark-memo android app, the notesviewmodel.init {} callback properly handles threading by dispa...Applied to files:
📚 Learning: in the ark-memo android app, textnotesrepo.findnote() intentionally returns null because text notes ...Applied to files:
🪛 detekt (1.23.8)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt[warning] 158-159: This empty block of code can be removed. (detekt.empty-blocks.EmptyFunctionBlock) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt (1)
173-186: Fix the incorrect command creation.The
AbsLineTocase is incorrectly usingSVGCommand.MoveTo.fromString(command)instead ofSVGCommand.AbsLineTo.fromString(command), which will create the wrong command type and cause incorrect SVG parsing behavior.Apply this diff to fix the bug:
SVGCommand.AbsLineTo.CODE -> { if (commandElements.size > 3) { strokeColor = commandElements[3] } if (commandElements.size > 4) { strokeSize = commandElements[4].toInt() } commands.addLast( - SVGCommand.MoveTo.fromString(command).apply { + SVGCommand.AbsLineTo.fromString(command).apply { paintColor = strokeColor brushSizeId = strokeSize }, ) }
🧹 Nitpick comments (1)
.editorconfig (1)
23-29: Verify IntelliJ-specific import-star settings
ij_kotlin_packages_to_use_import_on_demand = unsetis an IDE-internal key. In some older JetBrains versions “unset” is not recognized and silently falls back to the IDE default (usually 5). If the team has mixed IDE versions, consider pushing the value to something explicit like*or a large number for predictability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.editorconfig(1 hunks).github/workflows/build.yml(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferencesImpl.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt(5 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/FilePickerDialog.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/AboutFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt(6 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/BaseEditNoteFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/views/GraphicControlTextView.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/views/SettingTextView.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: in the ark-memo android app, the notesviewmodel.init {} callback properly handles threading by dispa...
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Applied to files:
app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/FilePickerDialog.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.ktapp/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferencesImpl.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/BaseEditNoteFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt
📚 Learning: in the ark-memo android app, textnotesrepo.findnote() intentionally returns null because text notes ...
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Applied to files:
app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/BaseEditNoteFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.ktapp/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt
🧬 Code Graph Analysis (1)
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.kt (2)
app/src/main/java/dev/arkbuilders/arkmemo/media/ArkMediaPlayer.kt (1)
isPlaying(27-27)app/src/main/java/dev/arkbuilders/arkmemo/media/ArkMediaPlayerImpl.kt (1)
isPlaying(75-75)
🪛 detekt (1.23.8)
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt
[warning] 157-158: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint
🔇 Additional comments (32)
.editorconfig (1)
1-14: Solid baseline style rules
The generic section covers the critical basics (indentation, newline, trimming, etc.) and will give your linters a single source of truth. Nice!app/src/main/java/dev/arkbuilders/arkmemo/ui/views/GraphicControlTextView.kt (1)
27-31: Readability improved – no functional diff
Switching to a multi-linegetColorcall with the trailing comma matches the new style guide and keeps long argument lists tidy.app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/AboutFragment.kt (1)
34-38: Consistent multi-line formatting
The expandedgetStringcall aligns with the.editorconfigmax-line-length rule and enhances clarity.app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/BaseEditNoteFragment.kt (1)
108-112: Style only, logic unchanged
Multi-line formatting here keeps the 100-char guideline intact while remaining readable. Looks good.app/src/main/java/dev/arkbuilders/arkmemo/ui/views/SettingTextView.kt (1)
28-32: Matches new formatting conventions
The wrappedgetBooleaninvocation is consistent with the rest of the refactor; no behavioural impact.app/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferencesImpl.kt (1)
37-41: LGTM! Excellent formatting improvement.The multi-line formatting of the
getCrashReportEnabled()method with trailing comma enhances readability and aligns with Kotlin style guidelines.app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.kt (2)
50-54: LGTM! Improved readability.The multi-line formatting of the
thumbViewWidthlazy property with trailing comma enhances code readability.
56-60: LGTM! Consistent formatting improvement.The multi-line formatting of the
thumbDirectorylazy property maintains consistency with the formatting style applied throughout the codebase.app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (1)
45-47: LGTM! Consistent formatting improvement.The multi-line formatting of the
newTextBuilder.append()call with trailing comma enhances readability and maintains consistency with the project's formatting standards.app/src/main/java/dev/arkbuilders/arkmemo/ui/dialogs/FilePickerDialog.kt (2)
23-24: LGTM! Clean annotation formatting.The multi-line formatting of the
@Injectannotation follows Kotlin conventions and improves readability.
71-75: LGTM! Improved method call readability.The multi-line formatting of the
ContextCompat.checkSelfPermission()call makes the parameters more readable while preserving the permission checking logic.app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.kt (2)
97-99: LGTM! Enhanced readability.The multi-line formatting of the
arkMediaPlayer.setMaxAmplitude()call with trailing comma improves code readability in the visualizer setup.
174-177: LGTM! Excellent multi-parameter formatting.The multi-line formatting of the
viewModelScope.launchPeriodicAsync()call with each parameter on its own line and trailing commas significantly improves readability of this complex method call.app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt (6)
131-131: LGTM!Good formatting improvement that enhances readability by visually separating the different XML tag handling cases.
134-138: LGTM!Excellent formatting improvement that makes the multi-parameter function call more readable and easier to maintain.
172-172: LGTM!Good formatting improvement that provides visual separation between different command handling cases.
187-187: LGTM!Good formatting improvement that maintains consistent visual separation between command handling cases.
188-201: LGTM!The
AbsQuadTocase is correctly implemented with proper command creation and consistent parameter extraction logic.
202-202: LGTM!Good formatting improvement that provides visual separation before the fallback
elsecase..github/workflows/build.yml (1)
1-131: Excellent workflow improvement!The restructuring to run linting jobs (
lintandktlint) before thebuildjob is a best practice that:
- Catches style and linting issues early in the pipeline
- Saves CI resources by preventing unnecessary builds when linting fails
- Aligns perfectly with the PR objective of enhancing code style and linting
The consistent 4-space indentation formatting also significantly improves readability.
app/src/main/java/dev/arkbuilders/arkmemo/ui/adapters/NotesListAdapter.kt (4)
38-42: LGTM!Excellent formatting improvement that makes the complex function type declaration much more readable and easier to understand.
73-78: LGTM!Good formatting improvement that makes the multi-parameter function call more readable and maintainable with proper indentation and trailing commas.
190-199: LGTM!Excellent formatting improvement that provides clear visual separation between different side effect handling cases, making the code much easier to read and understand.
311-339: LGTM!Great formatting improvements that make the complex conditional logic and when expression much more readable. The multi-line structure clearly shows the different note type handling branches.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt (4)
157-158: LGTM!The conversion from expression body to block body for this empty override method is fine. The static analysis warning about empty blocks is a false positive since this is a required
TextWatcherinterface method that intentionally has no implementation.
275-318: LGTM!Excellent formatting improvement that provides clear visual separation between different recording side effect handling cases, making the code much easier to read and follow.
327-350: LGTM!Good formatting improvement that maintains consistency with other similar methods and provides clear visual separation between different media player side effect handling cases.
336-340: LGTM!Good formatting improvement that makes the multi-parameter function call more readable with proper indentation and trailing commas.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt (4)
291-292: LGTM!Good formatting improvement that makes the complex conditional logic more readable and explicit with proper braces.
369-377: LGTM!Excellent formatting improvement that makes the complex nullable casting and property access much more readable and easier to understand.
447-451: LGTM!Good formatting improvement that makes the complex nullable chain of operations much more readable by breaking it into logical parts.
598-602: LGTM!Good formatting improvement that makes the multi-parameter function call more readable with each argument clearly visible on its own line.
Changes
Ref: https://app.asana.com/1/1207783906637200/project/1207819682524134/task/1210947771498078
Summary by CodeRabbit