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/cleanup make private #130

Merged
merged 9 commits into from
Dec 16, 2024
Merged

Chore/cleanup make private #130

merged 9 commits into from
Dec 16, 2024

Conversation

mattyg
Copy link
Member

@mattyg mattyg commented Dec 14, 2024

Some minor cleanup to this repo.

@@ -14,7 +14,7 @@ crate-type = ["cdylib", "rlib"]
path = "src/guest.rs"

[dependencies]
holochain_serialized_bytes = { version = "=0.0.55", features = [] }
holochain_serialized_bytes = "=0.0.55"
Copy link
Member Author

Choose a reason for hiding this comment

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

Cargo features are additive and this dep does not specify default-features = false, so I believe features = [] has no effect.

@@ -385,16 +383,6 @@ impl ModuleCache {
}
}

/// Configuration of a Target for wasmer for iOS
pub fn wasmer_ios_target() -> Target {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not used anywhere in holochain. If needed later we can restore it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Made several types private that are only used internally

./scripts/bench-wasmer_sys_dev.sh
./scripts/bench-wasmer_sys_prod.sh
./scripts/bench-wasmer_wamr.sh
Copy link
Member Author

@mattyg mattyg Dec 14, 2024

Choose a reason for hiding this comment

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

Now this script runs properly from the root directory only.

./scripts/test-wasmer_sys_dev.sh
./scripts/test-wasmer_sys_prod.sh
./scripts/test-wasmer_wamr.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

Now this script runs properly from the root directory only.

Copy link
Member Author

Choose a reason for hiding this comment

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

formatting

Copy link
Member Author

Choose a reason for hiding this comment

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

rename file to follow preferred convention for modules

@mattyg mattyg requested a review from a team December 14, 2024 19:07
@@ -104,12 +100,14 @@ pub trait PlruCache {

/// Delete the plru for a given cache key. Care must be taken to ensure this
/// is not called before a subsequent call to `plru_key` or it will panic.
#[allow(dead_code)]
fn trash(&mut self, key: &CacheKey) {
Copy link
Member

Choose a reason for hiding this comment

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

Are these really used? It's a private method on a private type now right?

If these are only used in tests I'd prefer #[cfg(test)] over dead_code

Copy link
Member Author

@mattyg mattyg Dec 16, 2024

Choose a reason for hiding this comment

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

No they're not used anywhere, though I thought they might be in the future if we do any pruning of the cache.

But they are very simple and can be added back if needed. Removed.

Copy link
Member

@ThetaSinner ThetaSinner left a comment

Choose a reason for hiding this comment

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

Nice! Just one question about dead code

@mattyg mattyg merged commit 3b0b4f3 into main Dec 16, 2024
13 checks passed
@mattyg mattyg deleted the chore/cleanup-make-private branch December 16, 2024 22:05
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