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

Are there certain dependencies that can be removed? #140

Closed
Milo123459 opened this issue May 26, 2022 · 10 comments · Fixed by #141 or #143
Closed

Are there certain dependencies that can be removed? #140

Milo123459 opened this issue May 26, 2022 · 10 comments · Fixed by #141 or #143
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Milo123459
Copy link
Contributor

Currently, moka has a lot of dependencies. I was wondering if there were any that could be replaced. I'd love to work on this.

@tatsuya6502
Copy link
Member

tatsuya6502 commented May 27, 2022

Hi. Thank you for the offer. Well, all these dependencies are in-use, of course. However if you only use moka::dash::Cache in serenity, you could remove the following dependencies from moka:

  • crossbeam-epoch
  • uuid
  • thiserror (but serenity already depends on it)

To do that, you will need the followings:

  1. Add a crate feature called sync to Cargo.toml, and add it to the default features.
  2. In Cargo.toml, make the following dependencies optional, and enable them only when sync or future feature is specified:
    • crossbeam-epoch
    • uuid
    • thiserror
  3. Make the following modules enabled only when sync feature is specified: (Use #[cfg(feature="sync")])
    • sync::builder
    • sync::cache
    • sync::segments
    • sync::value_initializer
  4. Make the following modules enabled only when sync or future feature is specified: (Use #[cfg(any(feature="sync", feature="future"))])
    • sync
    • cht
    • common::error

(I have not tried above by myself. So there may be additional changes required)

Once done, and a new version of moka is published to crate.io, you will update moka dependency in serenity to the followings:

serenity's Cargo.toml

[dependencies.moka]
version = "0.9"
default-features = false
features = ["dash", "atomic64", "quanta"]
optional = true

Also I found that scheduled-thread-pool v0.2.5 uses an older version of parking_lot (v0.11.2). serenity already depends on parking_lot but a newer version (v0.12.0). I see the master branch of sfackler/scheduled-thread-pool has it already upgraded to v0.12 via sfackler/scheduled-thread-pool#15, but no new version of scheduled-thread-pool has been published to crates.io.

So could you please open a GitHub issue to sfackler/scheduled-thread-pool and ask them to publish a new version to crates.io?

Thanks!

@tatsuya6502 tatsuya6502 added the enhancement New feature or request label May 27, 2022
@Milo123459
Copy link
Contributor Author

#141 made the pr!

@tatsuya6502
Copy link
Member

Thanks for making the PR #141. Hmm, there are so many compile errors.

After a brief review, I noticed a couple of things:

  1. moka::dash::Cache actually depends on the following crates, so we cannot remove them:
    • uuid
    • thiserror
  2. moka::dash::Cache actually depends on some modules under moka::sync, so we cannot simply disable then even though no sync or future feature are specified:
    • I would better to do some refactoring: e.g. move these shared modules under a new super module.

Let me take over your stuff and finish it. I will merge #141 into a topic branch and do the rest of the work.

@Milo123459
Copy link
Contributor Author

Are you sure you want to take it over?

Either way, thank you!

@tatsuya6502
Copy link
Member

Yeah. I will take it over as I will need to move some modules around. I merged #141 into a new topic branch gh141-optout-sync.

@tatsuya6502
Copy link
Member

tatsuya6502 commented May 28, 2022

I am doing some experiments here in gh141-experiments branch. It is almost working. I will continue working tomorrow as it is midnight now in my timezone.

I found the following is not true:

  • moka::dash::Cache actually depends on the following crates, so we cannot remove them:
    • uuid
    • thiserror

So I am able to remove uuid and thiserror crates when both sync and future features are not specified.

@tatsuya6502
Copy link
Member

tatsuya6502 commented May 29, 2022

OK. I have created two PRs:

  1. Refactoring: Reorganize internal modules #142
  2. Add a crate feature sync for enabling and disabling sync caches #143

I already merged #142, which was for moving some modules around.

#143 is for the sync feature and contains your commits and my commits. Once merged, disabling the sync feature (as well as future feature) will remove the following dependencies from moka:

  • crossbeam-epoch
  • uuid
  • thiserror

I will come back in a couple of hours to review the PR and will merge it into the next branch. This branch is for moka v0.9.0 release, while the master branch is for v0.8.x releases. I will merge #143 into next branch because it contains some breaking changes.

Please give me a couple of weeks to develop other stuff for v0.9.0 milestone.

@tatsuya6502
Copy link
Member

I reviewed and merged PR #143 into the next branch. I will keep this issue open until the PR gets merged into the master branch.

@tatsuya6502
Copy link
Member

Closing this issue as the next branch was merged into the master branch.

I will run some pre-release testing, and if everything goes well, I will publish Moka v0.9.0 to crates.io.

@tatsuya6502
Copy link
Member

@Milo123459 — I have published Moka v0.9.0 to crates.io 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants