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

Feature/allow single submissions #1852

Conversation

euberdeveloper
Copy link

Ref to #1445 and #1848, fixes #1850

@euberdeveloper
Copy link
Author

It fixes also this feature request: #1849.

The code probably is to be reviewed by the maintainers and re-adjusted, but it works as a proof of concept

Copy link

sonarqubecloud bot commented Jul 9, 2024

@tsaglam tsaglam added enhancement Issue/PR that involves features, improvements and other changes major Major issue/feature/contribution/change labels Jul 10, 2024
@tsaglam tsaglam requested a review from a team July 10, 2024 08:25
@tsaglam
Copy link
Member

tsaglam commented Jul 10, 2024

@euberdeveloper reviews will follow. For now, please take a look at the Sonar issues and see which ones you can resolve.

Copy link
Member

@Kr0nox Kr0nox left a comment

Choose a reason for hiding this comment

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

Could you add a simple e2e case to this pr, so we can make sure, it is always correctly exported.
Here are references on where they are implemented currently:
https://github.com/jplag/JPlag/blob/develop/report-viewer/tests/e2e/OpenComparisonTest.spec.ts
https://github.com/jplag/JPlag/blob/develop/.github/workflows/complete-e2e.yml

Please add one for every Problem solved by this PR

@euberdeveloper
Copy link
Author

Ok, I will do it next week

@tsaglam
Copy link
Member

tsaglam commented Jul 18, 2024

@euberdeveloper can you also extend the PR description so it becomes clearer what aspects this PR addresses and how it does so? Also how we can test it. We may need to discuss what features we want to include and which ones we don't want to include. Finally, please switch your target branch to develop, the main branch is only for releases.

@euberdeveloper
Copy link
Author

@tsaglam I've created a new branch that forks that:

  • branches develop instead of master
  • uses spotless to avoid style issues
  • adds tests that prove that the new behaviour works and work also as an example to understand it

Please take a look at it, I think that it is a good improvement that fixes #1849 and #1848 .

For a description of what it does, you can take a look at those issues (#1849 and #1848 ), namely:

  • If I want to test a single new submission against many old ones, it works instaed of throwing an error
  • If I have, for example, a new submission called "submission" and an old submission called also "submission", instead of throwing an error, requiring me to manually change the names of the folders, it gets as submission name the min. different path, e.g. "submissions/submission" and "olds/submission".
  • I find the old behaviour of throwing errors in those 2 cases limiting the tool and requiring users to find tricks to avoid those errors

The pull request will reference this old one, that I'm closing

@euberdeveloper euberdeveloper deleted the feature/allow-single-submissions branch September 6, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR that involves features, improvements and other changes major Major issue/feature/contribution/change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants