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

Remove unused dependencies #360

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kokosing
Copy link
Contributor

@kokosing kokosing commented Jul 26, 2024

Remove unused dependencies

@kokosing
Copy link
Contributor Author

@mneethiraj I have found these using mvn maven-dependency-plugin:analyze. I see other unused dependencies, so I will be posting other PRs like that shortly.

Would you like to grant me some access so CI could be executed for my PRs so I could see if my changes are working?

@kokosing
Copy link
Contributor Author

FYI @ksobolew

@kokosing kokosing changed the title Remove unused dependencies in unixauthclient Remove unused dependencies Jul 26, 2024
@kokosing kokosing force-pushed the origin/master/013_unused_deps branch from 4479b32 to c650d89 Compare July 26, 2024 21:31
Copy link
Contributor

@ksobolew ksobolew left a comment

Choose a reason for hiding this comment

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

It could have saved me some work ;)

@kokosing
Copy link
Contributor Author

I am not sure if it is the correct direction. I am not sure if I can trust mvn dependency:analyze here.

@kokosing
Copy link
Contributor Author

kokosing commented Aug 1, 2024

@mneethiraj do you think this is something I should invest my time in?

@mneethiraj
Copy link
Contributor

@kokosing - removal of unused dependencies will be of huge help; thank you for getting started on this. Can you please rebase this PR, so that I can trigger CI?

BTW, I recently started contributing to Trino; really liked Trino enforces several things during build time - like unused dependencies, duplicate classes, conflicting versions of dependencies, strict ordering of dependencies (in pom.xml). It will be awesome to get these in Ranger as well. Given your background in Trino, will you be able to help with these?

@kokosing kokosing force-pushed the origin/master/013_unused_deps branch from e8bf9ef to f25a888 Compare August 2, 2024 14:42
@kokosing
Copy link
Contributor Author

kokosing commented Aug 2, 2024

I have rebased the PR.

The way how it is done in Trino is an effect of huge effort over many years by multiple people. I am afraid I might not be able to migrate the entire Ranger project. The way it is done is via usage of https://github.com/airlift/airbase. It is easy to start a new project that is based on that, with existing project huge effort. However the biggest challenge I have with it is how to maek sure that changes I would be doing are safe to merge. It is easy to have compilation passing, but get errors on runtime. @mneethiraj do you believe that Ranger have good enough test coverage in CI that we move forward with it?

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.

3 participants