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

[#944] Add originality threshold flag #2122

Conversation

SkyBlaise99
Copy link
Contributor

Part of #944

Proposed commit message

The existing setup employs a static originality threshold of 0.51.
However, this threshold is tailored for codes, such as Java or Markdown,
and might not be suitable for other programming languages. Additionally,
it doesn't offer flexibility for users who may want a stricter threshold
but are willing to endure longer processing times, or those who prefer a
more lenient threshold but prioritize faster analysis speeds.

Let's enable users to input their preferred originality threshold.

Other information

jq1836 and others added 30 commits September 25, 2023 01:37
Currently, users are unable to select a zoom range that includes 
the until date.

This results in misleading data being presented to users.
…ense#2041)

Chrome bug is causing cypress to fail to open a browser on Github 
Actions, causing frontend tests and CI to fail. Upgrading cypress 
to greater than 12.15.0 will fix this issue.

Let's upgrade cypress to fix the failing CI.
Currently, there is still some JavaScript code which remains 
unmigrated. This allows for type unsafe code to be written, 
potentially resulting in unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript 
code to facilitate future changes to the code.
…posense#2040)

Currently, there is still some JavaScript code which remains 
unmigrated. This allows for type unsafe code to be written, 
potentially resulting in unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript 
code to facilitate future changes to the code.
Currently, Cypress zoom feature tests are failing due to a recent change
in behavior caused by a bug fix. With the tests failing, we are unable
to detect any future regressions.

Let's update the Cypress tests to test for the new intended behavior.
…#2043)

Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate random-color-generator.js JavaScript code to TypeScript
code to facilitate future changes to the code.
…sense#2036)

Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript code to
facilitate future changes to the code.
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript code to
facilitate future changes to the code.
Bumps [zod](https://github.com/colinhacks/zod) from 3.20.6 to 3.22.3.
- [Release notes](https://github.com/colinhacks/zod/releases)
- [Changelog](https://github.com/colinhacks/zod/blob/master/CHANGELOG.md)
- [Commits](colinhacks/zod@v3.20.6...v3.22.3)

---
updated-dependencies:
- dependency-name: zod
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@cypress/request](https://github.com/cypress-io/request) to 3.0.1 and updates ancestor dependency [cypress](https://github.com/cypress-io/cypress). These dependencies need to be updated together.


Updates `@cypress/request` from 2.88.12 to 3.0.1
- [Release notes](https://github.com/cypress-io/request/releases)
- [Changelog](https://github.com/cypress-io/request/blob/master/CHANGELOG.md)
- [Commits](cypress-io/request@v2.88.12...v3.0.1)

Updates `cypress` from 12.17.4 to 13.3.0
- [Release notes](https://github.com/cypress-io/cypress/releases)
- [Changelog](https://github.com/cypress-io/cypress/blob/develop/CHANGELOG.md)
- [Commits](cypress-io/cypress@v12.17.4...v13.3.0)

---
updated-dependencies:
- dependency-name: "@cypress/request"
  dependency-type: indirect
- dependency-name: cypress
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate the rest of the JavaScript code to TypeScript code to
facilitate future changes to the code.
Currently, when granularity is set to day or week, clicking on a ramp
will open up a zoom view where commit messages are not being displayed
and sorting by insertions does not result in any sorting. 

Let's fix the unintended behaviour of the zoom view.
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate repo-sorter.js to TypeScript code to facilitate future
changes to the code.
Currently, there is still some JavaScript code which remains unmigrated.
This allows for type unsafe code to be written, potentially resulting in
unintended behavior.

Let's migrate safari_date.js to TypeScript code to facilitate future
changes to the code.
Currently, frontend linter is failing due to lint scripts 
checking javascript files, the last of which has been 
removed in PR reposense#2053.

Lets update the lint command to exclude javascript 
files front the check.
…nse#2056)

Currently, most tooltips are shown above buttons and text. 
When these tooltips appear at the top of the viewport, 
part of the tooltips will not be rendered.

Let's implement changes such that these tooltips appear below the
text or button, when appearing at the top of the viewport.
… file title (reposense#2057)

Currently, if one hovers over a tooltip of the pinned title of
a file whose content is scrolled almost completely, such that 
the title of the next file is just below the pinned title, the 
tooltip is not displayed appropriately, as the title of the next 
file obstructs it.

Let's fix this issue.
…cs (reposense#2050)

There are still leftover references specific to GitHub on parts of 
the codebase and docs that have been generalized to accept 
other remote git hosts. 

Let's update these GitHub references to use more general language.
# Conflicts:
#	frontend/cypress/tests/chartView/chartView_zoomFeature.cy.js
#	frontend/src/views/c-authorship.vue
This reverts commit 950c912, reversing
changes made to 4bd05a7.
@SkyBlaise99 SkyBlaise99 marked this pull request as ready for review February 18, 2024 04:48
@ckcherry23 ckcherry23 requested a review from a team February 19, 2024 06:24
@ckcherry23 ckcherry23 requested a review from a team February 19, 2024 06:24
@ckcherry23
Copy link
Member

Hi @SkyBlaise99, would it be possible to merge/rebase with master so that the CI passes?

@SkyBlaise99
Copy link
Contributor Author

SkyBlaise99 commented Feb 19, 2024

iirc the backend side of the master branch was updated, the new introduction of builder pattern is quite a lot of change. I plan to make it a separate PR after these feature prs are merged to fix all the merge conflicts.

If everything is all together then this PR can be quite messy.

Anyways this PR is working on backend side so I hope the frontend failing tc shouldn't affect much.

Copy link
Member

@MarcusTXK MarcusTXK left a comment

Choose a reason for hiding this comment

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

LGTM, great job. I agree that resolving the merge conflict can be done later, especially as more changes to CliArguments.java are expected.

It would be great if you could address @ckcherry23 's comment for providing coverage of OriginalityThresholdArgumentType.java for lines 17, 20 and 23, but I think if its urgent to merge into #945, you could also do the testing there.

@MarcusTXK MarcusTXK requested a review from a team March 2, 2024 05:47
Copy link
Contributor

@gok99 gok99 left a comment

Choose a reason for hiding this comment

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

@SkyBlaise99 So sorry for the review delays!

Haven't taken a deep look at the full analyze-authorship changes, but within this PR's scope, changes are looking good to me. On that note, it would be great if you could come up with a simple reviewer guide (explaining key design decisions, changes to the overall architecture, etc) for facilitating the review of 944-analyze-authorship given the size of its scope.

Will mark this PR to be merged for now. Let us know if you intent to add tests to this PR before we merge it. If there is a particular hurry to get this merged, I think it's also fine to add the tests in the bigger PR.

@gok99 gok99 added s.ToMerge and removed s.ToReview labels Mar 2, 2024
@SkyBlaise99
Copy link
Contributor Author

On that note, it would be great if you could come up with a simple reviewer guide (explaining key design decisions, changes to the overall architecture, etc) for facilitating the review of 944-analyze-authorship given the size of its scope.

Imo there isn't actually key design decisions or changes to overall architecture. This is more like a add-on feature to improve the existing code authorship attribution. Nevertheless, I'll provide a rough explanation as to what this code does when the final PR is made.

Will mark this PR to be merged for now. Let us know if you intent to add tests to this PR before we merge it.

I can add in the test in this PR to make it more complete. It should be a minor update.

@gok99 gok99 removed the s.ToMerge label Mar 2, 2024
@gok99 gok99 added the s.ToMerge label Mar 3, 2024
@gok99 gok99 merged commit 79209a3 into reposense:944-analyze-authorship Mar 3, 2024
9 of 10 checks passed
Copy link
Contributor

github-actions bot commented Mar 3, 2024

The following links are for previewing this pull request:

@gok99
Copy link
Contributor

gok99 commented Mar 3, 2024

Merging this and #2108 ahead of the 2 day window since we're merging to an intermediate branch.

@SkyBlaise99 SkyBlaise99 deleted the 944-originality-threshold-flag branch March 3, 2024 07:00
@SkyBlaise99 SkyBlaise99 restored the 944-originality-threshold-flag branch March 3, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants