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

[Refactoring] Code clarifications, Unit Tests, Responsibilities #109

Merged
merged 10 commits into from
Mar 29, 2022

Conversation

afillatre
Copy link
Contributor

In order to go further with the project I had the need to better understand the process behind dependency exploration and consolidation. I rewrote a great part of the depclean-core for it to be easier to understand, more testable, and with better responsibility segregation (or so I hope).

Also I added some logs that also make it clearer what happens in the process.

The DeclaredDependencyGraph calculate the dependencies and their related classes. The ActualUsedClasses collects the classes that actual are used in the project, and that match detected classes from the dependencies. Then, the ProjectDependencyAnalysisBuilder prepares the analysis result to be passed to the Maven Plugin.

I think much work is still needed since some code is still out of place in the Mojo rather than in the core (for responsibility and testability), but I hope this is a good first step.

@codecov
Copy link

codecov bot commented Mar 19, 2022

Codecov Report

Merging #109 (ab671a3) into master (662d116) will increase coverage by 16.79%.
The diff coverage is 33.81%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master     #109       +/-   ##
=============================================
+ Coverage     23.17%   39.96%   +16.79%     
- Complexity       86      178       +92     
=============================================
  Files            29       40       +11     
  Lines          1027     1126       +99     
  Branches        138       84       -54     
=============================================
+ Hits            238      450      +212     
+ Misses          758      635      -123     
- Partials         31       41       +10     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 662d116...ab671a3. Read the comment docs.

@afillatre
Copy link
Contributor Author

I'm actually working on a second version that completely removes the dependency to Maven in the core, so the core will be even more testable and integrate better with Gradle (no need to duplicate business code in the plugin projects)

@afillatre
Copy link
Contributor Author

@cesarsotovalero I think I'm done for the most part. Please feel free to tell me if you like this refactoring or not, and the things you want me to change/improve.

For this second part, I totally removed the maven coupling from the core, and moved some logic into there. The idea behind that is that the plugin (maven or gradle) provides the right context for the core to execute, then exploit the result. Some more things may need to be moved to the core later, though.

Also, I ran the depclean on itself, replaced the poms with the deblaoted ones, and the project still works with no more unused dependencies, so I hope the refactoring didn't break anything.

I did not work on the Gradle project yet, so this may be broken at the moment.

@afillatre
Copy link
Contributor Author

There is actually an issue with the debloated pom (rewrite is not working as expected), so I'll look into that.

@sonarcloud
Copy link

sonarcloud bot commented Mar 29, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 24 Code Smells

27.1% 27.1% Coverage
0.0% 0.0% Duplication

@cesarsotovalero
Copy link
Collaborator

Hi @afillatre,
Thanks for these refactorings!
I think that this separation of concerns between the functionalities that are part of the analysis and the entities that are part of the new model package makes it easy to understand.
I've fixed the Gradle plugin accordingly.
I'll merge this PR.
Please open another PR if you want to add a new feature to DepClean 🚀

@cesarsotovalero cesarsotovalero merged commit b970294 into ASSERT-KTH:master Mar 29, 2022
@afillatre afillatre deleted the refacto_code-cleanup branch March 31, 2022 17:02
@afillatre
Copy link
Contributor Author

@cesarsotovalero thanks for accepting it ! I still have some improvements to add, and some proposals to make

afillatre pushed a commit to afillatre/depclean that referenced this pull request Mar 31, 2022
…RT-KTH#109)

* [Refactoring] Code clarifications, Unit Tests, Responsibilities

* More cleanup

* Remove maven dependencies from core

* Cleanup and use debloated poms

* De-duplicate dependencies before generating debloated pom

* More refactoring into separate classes.

* Move logic in the core, so it's independent from a specific dependency manager

* Fix Gradle plugin

* Try updating JDK to 17

* Change gradle build in GitHub actions to JDK 11

Co-authored-by: César Soto Valero <cesarsotovalero@gmail.com>
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

2 participants