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

chore: dependency cleanup #1150

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

milenkovicm
Copy link
Contributor

Which issue does this PR close?

Closes none.

Rationale for this change

There are a bit too many dependencies, some of which are duplicated and can be replaced with some other we use, some of which are not maintained. As users can create their own binaries there is no need to depend on dependencies which our binaries use.

There were cases where log and trace has been used in the same files, I have removed trace but we can revisit that decision in subsequent PRs

What changes are included in this PR?

  • remove few dependencies (anyhow, parse_arg)
  • make few dependencies optional. dependencies used in executor and scheduler binaries are optional now
  • remove use of trace in few files where log was used as well

Are there any user-facing changes?

No

@milenkovicm milenkovicm marked this pull request as draft December 11, 2024 09:06
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you help me understand why we are removing the tracing? I know you have a good reason, just I am missing the context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we use tracing we use log, we need to pick one and stick with it. log is use across project, so i remove those few statements which used tracing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whole point of this PR is to decrease number of dependencies used in core components

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation!

@milenkovicm milenkovicm marked this pull request as ready for review December 11, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants