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] Resolve Merge Conflict #2139

Merged
merged 101 commits into from
Mar 7, 2024

Conversation

SkyBlaise99
Copy link
Contributor

@SkyBlaise99 SkyBlaise99 commented Mar 3, 2024

Part of #944

Proposed commit message

Currently the branch is ready but out of sync with the master branch,
which had some major revamp in both the frontend and backend.

Let's merge the master branch into this branch to keep it up to date.

Other information

This PR has no new feature, purely just for updating and resolving merge conflicts.

SkyBlaise99 and others added 30 commits September 12, 2023 21:45
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.
georgetayqy and others added 21 commits February 7, 2024 01:28
…f classes (reposense#2104)

Classes in the `parser` package as of now are not properly organised,
with exceptions, types and parser classes being found in the same
directory level.

This does not follow the convention established by other packages,
whereby logically similar classes are further grouped into
sub-packages.

Let's move to refactor the `parser` package to ensure that it follows
the overall structure of the other packages and to increase the level
of organisation within the `parser` package.
…osense#2107)

Fix broken link to DevOps Guide in Learning Basics page.

Under the DevOps section in the developer guide,
the link to DevOps guide leads a Wiki link instead of redirecting to
the DevOps guide on the actual Reposense website.

Let's fix the broken link by changing the address to the correct page.
Fix nondeterministically failing zoomFeature cypress test

The "range changes in chartview should reflect in zoom" test in
chartView_zoomFeature.cy.js fails because as time passes, the
coordinates in the ramp that correspond to the desired zoom area
change.

Let's add an "until" filter to the relevant cypress tests to stop this
from happening
…omplexity (reposense#2078)

Refactor RepoConfiguration to simplify constructor complexity.

Currently, the constructors for the `RepoConfiguration` class is 
very complex, involving up to 18 parameters at once, and is 
not easily extensible.

With the proposed changes, the constructor now follows the 
Builder pattern, by using a nested class `Builder` that 
helps to compose config parameters in a way that is much more 
modular and comprehensible, enhancing code quality and 
reducing confusion among developers.

However, it must be noted that `Builder` objects are not reusable as of
now, and that more combinations of arguments are permitted due to the
removal of constructors for `RepoBuilder`, which previously helped to
ensure the correctness of parameter combinations.

Let's refactor `RepoConfiguration` to simplify the 
complexity of the constructor and increase modularity 
of the code.
Implement Title Component

With a new focus on allowing users to use RepoSense as a portfolio
tool, more functionality supporting this focus is needed.

Let's allow users to add customizable content in Markdown/HTML format 
at the top of the report for a personalized introduction.
The segment component CSS currently resides in the c-authorship-file
component. Moving it to the segment component would make it easier to
edit the segment component in the future.

Let's move the CSS into c-segment.vue.
# Conflicts:
#	frontend/src/utils/segment.ts
#	frontend/src/views/c-authorship.vue
#	src/main/java/reposense/parser/ArgsParser.java
#	src/main/java/reposense/util/StringsUtil.java
…e#2135)

The code highlighting is not working as the colors are not being
recognized by the newly extracted c-segment.vue file. This is because
the defined colors were not imported into the file.  

Let us add the color styles import to restore code highlighting
functionality.
Fix Blurry Favicon on Reports

The favicon looks blurry on generated reports.

Let's replace this with a clearer image.
…tion`'s Builder Pattern (reposense#2118)

Current implementation of the Builder pattern within `CliArguments`
does not conform that in `RepoConfiguration`.

Let's move to refactor `CliArguments` to reduce class attribute
duplication across `CliArguments` and `CliArguments::Builder` and to
enhance overall code quality.
# Conflicts:
#	src/main/java/reposense/parser/ArgsParser.java
# Conflicts:
#	src/main/java/reposense/model/CliArguments.java
@SkyBlaise99 SkyBlaise99 marked this pull request as ready for review March 3, 2024 08:02
@gok99
Copy link
Contributor

gok99 commented Mar 3, 2024

It is a little hard to tell what conflicts are resolved in this PR as it is. Would it be possible to mark out where you've had to resolve conflicts, perhaps with the original changes before the conflict was resolved?

@SkyBlaise99
Copy link
Contributor Author

SkyBlaise99 commented Mar 4, 2024

@gok99 Here's a short summary of the merge conflicts. Hope this helps a bit.

Conflict Commit Remark
frontend/src/utils/segment.ts 83510f1 Removed
frontend/src/views/c-authorship.vue 83510f1 Followed updates from master. Lines 864 - 1042 are relocated to lines 75 - 151 in c-segment.vue.
src/main/java/reposense/model/CliArguments.java ef320c0 Added lines 38, 39, 479 and 489. The rest followed updates from master
src/main/java/reposense/parser/ArgsParser.java 83510f1 Added lines 314, 315, 335 and 336. The rest followed updates from master
src/main/java/reposense/util/StringsUtil.java 83510f1 Followed updates from master

I think it is easier to just merge this and look at the diff of 2 branches instead. A sample one is available here. The merge conflicts are simply relocating my codes to the new places as the old file/structures are changed. (frontend is splitting the code into smaller files and backend has updated the builder pattern).

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.

That's very helpful, thanks! Backend changes look good

Copy link
Member

@ckcherry23 ckcherry23 left a comment

Choose a reason for hiding this comment

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

Thanks a lot @SkyBlaise99 for creating #2140 to view the diff. Frontend changes look good to me.

I may have a few simple code quality suggestions we could tackle in another PR:

  • Add a comment to line 22 in c-segment.vue to show that the author color is applied only when the author color exists, else it takes the default mui color value (This was not your change, but since there is a lot of highlight color logic now, this could help).
  • Define a type for lastState in line 413 of c-authorship.vue that includes both the last known author's name and the last line's full credit status for easier comparison, if possible.

@SkyBlaise99
Copy link
Contributor Author

SkyBlaise99 commented Mar 7, 2024

Thanks for the feedbacks. For those code quality suggestions, I think they will be easier to work on after the feature branch is settled.

Can this PR be merged so I can put a new PR to merge the feature branch into master like #2140? Thanks.

@gok99 gok99 merged commit b90b9a3 into reposense:944-analyze-authorship Mar 7, 2024
18 checks passed
Copy link
Contributor

github-actions bot commented Mar 7, 2024

The following links are for previewing this pull request:

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.