-
Notifications
You must be signed in to change notification settings - Fork 332
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
fix bug: representation for name of file and match #785
Conversation
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.
First of all: Thank you for your fast first PR, looks promising.
Here are some initial hints regarding the code quality. Please also fix the merge conflicts so that I can test the new functionality.
core/src/main/java/de/jplag/reporting/jsonfactory/ComparisonReportWriter.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/reportobject/ReportObjectFactory.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/reportobject/ReportObjectFactory.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/reportobject/ReportObjectFactory.java
Outdated
Show resolved
Hide resolved
8d1c3a2
to
6a5ce87
Compare
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.
A few issues regarding code quality. Also, Sonar found one code smell.
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/jsonfactory/DirectoryManager.java
Outdated
Show resolved
Hide resolved
core/src/test/java/de/jplag/reporting/reportobject/ReportObjectFactoryTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/jsonfactory/DirectoryManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/jsonfactory/DirectoryManager.java
Outdated
Show resolved
Hide resolved
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.
Just a few minor issues.
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/de/jplag/reporting/reportobject/ReportObjectFactoryTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/jsonfactory/DirectoryManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/de/jplag/reporting/jsonfactory/ComparisonReportWriter.java
Outdated
Show resolved
Hide resolved
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.
I am sorry, but the previous changes decreased the quality of the test case by a lot...
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
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.
I rewrote your test case as a pointer of how we would expect good test cases.
core/src/test/java/de/jplag/reporting/jsonfactory/DirectoryManagerTest.java
Outdated
Show resolved
Hide resolved
…agerTest.java Co-authored-by: Timur Sağlam <[email protected]>
Kudos, SonarCloud Quality Gate passed! |
I have changed the submissions's folder structure and json file of comparison(i.e. A-B.json) in zip file. Now match and file name are represented with their full path (include root directory). And the problem in #658 is also solved. And when file name is too long, it only shows part of the full path(i.e. ..subfolder/subsubfolder/algorithm.java).