Skip to content

feat(forge build): cache project selectors by default #10651

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

Merged

Conversation

mablr
Copy link
Contributor

@mablr mablr commented May 28, 2025

Motivation

Resolve #10640

Solution

Calls cache_local_signatures function like the "forge selectors cache" command.

Remark

cache_local_signatures function has been updated to accept an optional cache_dir, no more need to unwrap() the option value returned foundry's config crate.

PR Checklist

  • Added Tests (reused unit test of "forge selectors cache")
  • Added Documentation
  • Breaking changes

@mablr mablr force-pushed the feature/forge-build-cache-selectors branch from b28c4e5 to cda756f Compare May 28, 2025 09:41
Calls `cache_local_signatures` function like the "forge selectors cache" command.
@mablr mablr force-pushed the feature/forge-build-cache-selectors branch from cda756f to a70c367 Compare May 28, 2025 11:54
@mablr mablr changed the title feat(forge build): add --selectors_cache option feat(forge build): cache project selectors by default May 28, 2025
zerosnacks
zerosnacks previously approved these changes May 28, 2025
Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

Thanks! Confirming the test correctly asserts, without caching the selectors it fails to decode

…ptional `cache_dir`

No more need to unwrap the option value returned  foundry's `config` crate
@mablr mablr force-pushed the feature/forge-build-cache-selectors branch from b519bf6 to fd92f8c Compare May 28, 2025 20:23
let Some(cache_dir) = cache_dir else {
eyre::bail!("Failed to get `cache_dir` to generate local signatures.");
};

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 updated cache_local_signatures in a separate commit, squash it if you think it's not necessary.

@mablr mablr requested a review from zerosnacks May 28, 2025 20:38
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

thank you, lgtm! Made a small nit re cache dir in cache sigs fns

@grandizzy grandizzy enabled auto-merge (squash) May 30, 2025 12:59
@grandizzy grandizzy merged commit 197586f into foundry-rs:master May 30, 2025
61 of 66 checks passed
@github-project-automation github-project-automation bot moved this to Done in Foundry May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

feat(forge build): automatically cache project selectors upon running
4 participants