Skip to content

CodeRabbit Review #7

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.

Sign up for GitHub

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

Open
kngys wants to merge 60 commits into
base: coderabbit
Choose a base branch
from
Open

CodeRabbit Review #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
759f996
Level 0. Rename, Greet, Exit
kngys Aug 19, 2024
9268841
Level 1. Echo
kngys Aug 19, 2024
0dbc456
Update to accept user input as a phrase instead of only a single word
kngys Aug 19, 2024
62182fd
Level 2. Add, List
kngys Aug 19, 2024
a12e40f
Add Task class
kngys Aug 20, 2024
badf6e1
Level 3. Mark as Done
kngys Aug 20, 2024
525c06e
Add Deadline class
kngys Aug 20, 2024
e83758a
Add Todo class
kngys Aug 20, 2024
baf2a15
Add Event class
kngys Aug 20, 2024
5523e4a
Level 4. ToDos, Events, Deadlines
kngys Aug 20, 2024
818719b
A-TextUiTesting
kngys Aug 21, 2024
03c1056
Level 5. Handle Errors
kngys Aug 22, 2024
5d9b1a7
Level 6. Delete
kngys Aug 22, 2024
facd46a
Add method to format saving of tasks
kngys Aug 27, 2024
dfe3bbe
Add feature to save tasks
kngys Aug 27, 2024
a30e1bb
Merge branch 'branch-Level-7'
kngys Aug 27, 2024
df907a2
Store deadline dates as LocalDate and format date
kngys Aug 28, 2024
ea0c591
Merge branch 'branch-Level-8'
kngys Aug 28, 2024
5b160c6
Add Ui, Storage, Parser and TaskList classes
kngys Aug 29, 2024
97c50d7
Update logic of main class
kngys Aug 29, 2024
2d7c03c
Merge branch 'branch-A-MoreOOP'
kngys Aug 29, 2024
8353048
Put all classes in one package
kngys Aug 29, 2024
59fc225
Merge branch 'branch-A-Packages'
kngys Aug 29, 2024
ef7271d
Merge branch 'add-gradle-support'
kngys Aug 29, 2024
8bc28ff
Add Gradle
kngys Aug 29, 2024
d07dcc8
Add JUnit tests to test Todo, TaskList and Storage classes
kngys Aug 29, 2024
4f6852e
Merge branch 'branch-A-JUnit'
kngys Aug 29, 2024
750a6bd
Package app as JAR file
kngys Aug 29, 2024
b67f73f
Add JavaDoc comments
kngys Aug 29, 2024
8eddbed
Update to follow coding standard
kngys Aug 29, 2024
3b93b9a
Add find function
kngys Aug 29, 2024
352e901
Merge branch 'branch-A-JavaDoc'
kngys Aug 29, 2024
c5854ad
Merge branch 'branch-A-CodingStandard'
kngys Aug 29, 2024
9714d34
Merge branch 'branch-Level-9'
kngys Aug 29, 2024
ca19d4f
Update JavaDoc comments
kngys Sep 4, 2024
43e435a
Add checkstyle configurations
kngys Sep 4, 2024
11de21f
Update to follow coding standard
kngys Sep 4, 2024
56d2074
Merge branch 'branch-A-CheckStyle'
kngys Sep 4, 2024
2df1e73
Add GUI functionality
kngys Sep 6, 2024
01664b9
Merge branch 'branch-Level-10'
kngys Sep 6, 2024
f95f542
Add assertions
kngys Sep 12, 2024
02375d4
Improve code quality
kngys Sep 12, 2024
bde1fce
Merge pull request #1 from kngys/branch-A-Assertions
kngys Sep 12, 2024
4cd32f5
Merge branch 'master' into branch-A-CodeQuality
kngys Sep 12, 2024
7a2c45d
Merge pull request #2 from kngys/branch-A-CodeQuality
kngys Sep 12, 2024
8ba8871
Add functionality to edit task details
kngys Sep 16, 2024
5a99c81
Merge branch 'branch-C-Update'
kngys Sep 16, 2024
0614488
Update GUI
kngys Sep 19, 2024
ce8263f
Add user guide
kngys Sep 19, 2024
f342cba
Update user guide
kngys Sep 19, 2024
3aec0eb
Update user guide
kngys Sep 19, 2024
c03b5e1
Update JavaDoc comments and exception handling
kngys Sep 20, 2024
35dbd3f
Update GUI and JUnit tests
kngys Sep 20, 2024
36bc076
Update user guide
kngys Sep 20, 2024
6615596
Improve exception handling
kngys Sep 20, 2024
b5e81ae
Improve code quality
kngys Sep 20, 2024
b73902f
Update user guide
kngys Sep 20, 2024
24eb5ab
Improve exception handling
kngys Sep 20, 2024
e06a0c4
Update user guide
kngys Sep 20, 2024
ac6329a
Improve code quality
kngys Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ plugins {
id 'java'
id 'application'
id 'com.github.johnrengelman.shadow' version '7.1.2'
id 'checkstyle'
}

repositories {
Expand All @@ -11,6 +12,20 @@ repositories {
dependencies {
testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: '5.10.0'
testRuntimeOnly group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: '5.10.0'
String javaFxVersion = '17.0.7'

implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'win'
implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'mac'
implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'linux'
implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'win'
implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'mac'
implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'linux'
implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'win'
implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'mac'
implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'linux'
implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'win'
implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'mac'
implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'linux'
Comment on lines +15 to +28

@coderabbitai coderabbitai Bot Sep 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid bundling all OS JavaFX artifacts; use the OpenJFX Gradle plugin.

Shading win/mac/linux variants together bloats the JAR and can cause native clashes. Let the plugin resolve platform-specific artifacts at build/run time.

-    String javaFxVersion = '17.0.7'
-
-    implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'win'
-    implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'mac'
-    implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'linux'
-    implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'win'
-    implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'mac'
-    implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'linux'
-    implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'win'
-    implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'mac'
-    implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'linux'
-    implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'win'
-    implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'mac'
-    implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'linux'
+    // JavaFX dependencies are managed by the javafx {} block below
-plugins {
+plugins {
     id 'java'
     id 'application'
     id 'com.github.johnrengelman.shadow' version '7.1.2'
     id 'checkstyle'
+    id 'org.openjfx.javafxplugin' version '0.0.14'
 }

Add near the application block:

javafx {
    version = '17.0.7'
    modules = ['javafx.controls', 'javafx.fxml']
}

Optional: pin toolchain to 17 for consistency with JavaFX 17.

java {
  toolchain { languageVersion = JavaLanguageVersion.of(17) }
}
🤖 Prompt for AI Agents 
In build.gradle around lines 15-28, remove the per-OS javafx implementation
dependencies (the win/mac/linux classifier lines) and instead apply the OpenJFX
Gradle plugin and declare the javafx configuration in the application block so
platform artifacts are resolved at build/run time; add a javafx block setting
version = '17.0.7' and modules = ['javafx.controls','javafx.fxml'], and
optionally pin the Java toolchain to Java 17 (java.toolchain languageVersion =
17) for consistency with JavaFX 17.

}

test {
Expand All @@ -28,14 +43,20 @@ test {
}

application {
mainClass.set("seedu.duke.Duke")
mainClass.set("serenity.gui.Launcher")
}

shadowJar {
archiveBaseName = "duke"
archiveBaseName = "serenity"
archiveClassifier = null
archiveFileName = 'serenity.jar'
}
Comment on lines 49 to +53

@coderabbitai coderabbitai Bot Sep 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Set Main-Class in shaded JAR manifest.

Ensures java -jar serenity.jar runs without extra args.

 shadowJar {
     archiveBaseName = "serenity"
     archiveClassifier = null
     archiveFileName = 'serenity.jar'
+    manifest {
+        attributes 'Main-Class': 'serenity.gui.Launcher'
+    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
shadowJar {
archiveBaseName = "duke"
archiveBaseName = "serenity"
archiveClassifier = null
archiveFileName = 'serenity.jar'
}
shadowJar {
archiveBaseName = "serenity"
archiveClassifier = null
archiveFileName = 'serenity.jar'
manifest {
attributes 'Main-Class': 'serenity.gui.Launcher'
}
}
🤖 Prompt for AI Agents 
In build.gradle around lines 49–53, the shadowJar block doesn't set a Main-Class
in the JAR manifest so running "java -jar serenity.jar" fails; update the
shadowJar configuration to add a manifest entry "Main-Class" pointing to your
application's fully-qualified main class (e.g. com.yourorg.Main), ensuring the
application/mainClassName (or application.plugin) is set consistently, and
re-run the shadowJar task so the produced serenity.jar contains that Main-Class
manifest attribute.


checkstyle {
toolVersion = '10.2'
}

run{
standardInput = System.in
enableAssertions = true;
}
Loading