Skip to content
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

Merged
merged 16 commits into from
Nov 29, 2022

Conversation

cyfml
Copy link
Contributor

@cyfml cyfml commented Nov 13, 2022

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).

@tsaglam tsaglam requested a review from sebinside November 14, 2022 07:38
@tsaglam tsaglam added bug Issue/PR that involves a bug report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies labels Nov 14, 2022
Copy link
Member

@sebinside sebinside left a 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.

@cyfml cyfml closed this Nov 17, 2022
@tsaglam tsaglam changed the title fixed first bug in #710. fix bug: representation for name of file and match Nov 18, 2022
@sebinside sebinside self-requested a review November 19, 2022 09:38
Copy link
Member

@tsaglam tsaglam left a 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.

@sebinside sebinside self-requested a review November 25, 2022 14:06
Copy link
Member

@tsaglam tsaglam left a 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.

Copy link
Member

@tsaglam tsaglam left a 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...

Copy link
Member

@tsaglam tsaglam left a 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.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

71.8% 71.8% Coverage
0.0% 0.0% Duplication

@tsaglam tsaglam merged commit 3c02c0c into jplag:develop Nov 29, 2022
@tsaglam tsaglam added the minor Minor issue/feature/contribution/change label Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue/PR that involves a bug minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants