-
Notifications
You must be signed in to change notification settings - Fork 590
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
[Ernest Yu] iP #619
base: master
Are you sure you want to change the base?
[Ernest Yu] iP #619
Conversation
In build.gradle, the dependencies on distZip and/or distTar causes the shadowJar task to generate a second JAR file for which the mainClass.set("seedu.duke.Duke") does not take effect. Hence, this additional JAR file cannot be run. For this product, there is no need to generate a second JAR file to begin with. Let's remove this dependency from the build.gradle to prevent the shadowJar task from generating the extra JAR file.
private File file; | ||
private final String filePath; | ||
|
||
Storage(String filePath) { |
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.
Perhaps an access modifier could be added for the constructor here for the sake of completeness :)
String[] arr = desc.split("/from"); | ||
String[] arr2; |
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.
These could have better naming :) (though I would be too lazy too most of the times ngl)
src/main/java/chatBot/bot/Ui.java
Outdated
Ui() { | ||
this.scanner = new Scanner(System.in); | ||
} | ||
|
||
void showWelcome() { | ||
System.out.print(HLINE + | ||
" Hello! I'm 'B word'\n" + | ||
" What can I do for you?\n" + | ||
HLINE); | ||
} |
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.
Could consider proper access modifiers for most methods in this class
Merge branch-A-Assertion to Master
Attempt to merge branch onto master
Merge Branch-A-Streams
Bword
My services:
USE me with
find me at here (well im not providing the link duh 🙄 )
Free (just for you) 😉
this is an inline code