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

DepClean Gradle Plugin implementation. #93

Closed
wants to merge 34 commits into from

Conversation

ABHAY0O7
Copy link
Contributor

@ABHAY0O7 ABHAY0O7 commented Jun 3, 2021

Currently, I think I have implemented the core logic correctly, but still, there are some drawbacks that I am still trying to improve.
Also, the code that I have written until now is very unmanaged, there are lots of unnecessary commented out code, there are lots of method that I used for debugging and I provided very few java docs, so sorry for that. Please ignore these parts of code.
Currently to test the plugin,

  1. First, Run gradle clean build without changing anything,
  2. Run gradle publishToMavenLocal
  3. Now uncomment the buildscript and apply plugin part that I have commented in build.gradle and again run gradle clean build
  4. Run gradle copyDependenciesLocally task.
  5. Run final gradle debloat task and you can see the results.

Thank you

@ABHAY0O7
Copy link
Contributor Author

ABHAY0O7 commented Jun 3, 2021

Hello @tdurieux @bbaudry
I was thinking that if we could have a meet this weekend or may after that so that I can discuss the current status of the plugin, and also some problems that I am facing right now. The problems are quite complicated which I think can only be illustrated better at a meet, so it will be better for me.

@ABHAY0O7 ABHAY0O7 mentioned this pull request Jun 9, 2021
Copy link
Contributor

@tdurieux tdurieux 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 remove all the Gradle folders, we would like to avoid to have .ar files inside the project?

Could you remove all the .gradle folder from the test resources?

Could you rename the folder depclean-gradle-plugin/src/Test to test?

It is for the moment difficult to review the changes when so many files are included in the PR. After you remove the folders I will continue to review the PR

@ABHAY0O7
Copy link
Contributor Author

Could you remove all the .gradle folder from the test resources?

I already removed them but I forgot to remove their caches, but now I removed them too.

Could you rename the folder depclean-gradle-plugin/src/Test to test?

Renamed 👍🏽

Could you remove all the Gradle folders, we would like to avoid to have .ar files inside the project?

Are you talking about gradle.wrapper folders? If no, then please can you be more specific but If yes then in my knowledge we should not involve these folders in .gitignore as they are important ones. But if you want these folders to be ignored for now then I can remove them too. So please can you be more specific about this question that what I have to do.

@ABHAY0O7 ABHAY0O7 changed the title Gradle Plugin core implemented. DepClean Gradle Plugin implementation. Jul 4, 2021
@ABHAY0O7 ABHAY0O7 requested a review from tdurieux July 4, 2021 11:08
Copy link
Contributor

@tdurieux tdurieux left a comment

Choose a reason for hiding this comment

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

Good job, the PR seems good. There are few elements that need to improve but it should be relatively fast.

depclean-gradle-plugin/build.gradle Outdated Show resolved Hide resolved
/**
* If true, the project's classes in target/test-classes are not going to be analyzed.
*/
private final boolean isIgnoredTest;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you create the same parameters that we have in the maven plugins? (could be done in a separate PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added all the parameters that we have in the maven plugin except for 3 parameters about which I talked about here
Please take a look there and then reply accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would still implement those parameters, we can have set the default to false for those additional parameters but those are useful.

@ABHAY0O7
Copy link
Contributor Author

ABHAY0O7 commented Jul 7, 2021

Hello @tdurieux
I have implemented all the requested changes except for one which is still unresolved.
To resolve them, you have to be more clear that what I have to do.

Thank you.

@ABHAY0O7 ABHAY0O7 requested a review from tdurieux July 18, 2021 00:51
@tdurieux
Copy link
Contributor

Looks good. depclean-gradle-plugin/src/main/java/se/kth/depclean/DepCleanGradleAction.java is really big maybe we should work on a way to make it smaller and easier to maintain.

@ABHAY0O7
Copy link
Contributor Author

Looks good. depclean-gradle-plugin/src/main/java/se/kth/depclean/DepCleanGradleAction.java is really big maybe we should work on a way to make it smaller and easier to maintain.

Hii @tdurieux,
As you suggested I reduced the size of the DepCleanGradleAction, to do this I introduce two new utility classes.
I also used the Path object instead of String path which saves a little bit extra lines of code.

@tdurieux
Copy link
Contributor

I just realized that Github Action does not execute the test from the gradle plugin. Could you update https://github.com/castor-software/depclean/blob/master/.github/workflows/build.yml to include the gradle plugin tests.
I think it is the final task before I can merge the PR.

It is a good job, continue like this!

@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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ABHAY0O7
Copy link
Contributor Author

Hii @tdurieux
I want you to please don't merge this PR for now.
This is PR is very messy and there are some useless commits also, so to make the history very clean and easily understandable, give me some time, I will update you soon.

Thank you

@ABHAY0O7 ABHAY0O7 changed the title DepClean Gradle Plugin implementation. DepClean Gradle Plugin implementation. (Not for merge) Jul 27, 2021
@ABHAY0O7
Copy link
Contributor Author

Hii @tdurieux
I have started to make the git history clean and understandable.
I have opened some PR's with their order of merge in their titles, now the commits are more clear and understandable with some description.
I want you to review and merge the PRs as soon as possible, so that when I push new commits to my branches then they should be on new PR. :))

Thank you
Abhay

@ABHAY0O7
Copy link
Contributor Author

Hii @tdurieux
Nice to see you :))
I will update you with remaining PRs up to 7 PM GMT.

Thank you

@ABHAY0O7 ABHAY0O7 closed this Jul 29, 2021
@ABHAY0O7 ABHAY0O7 deleted the My_Gradle branch August 12, 2021 18:51
@ABHAY0O7 ABHAY0O7 changed the title DepClean Gradle Plugin implementation. (Not for merge) DepClean Gradle Plugin implementation. Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants