fix(forge): disable artifacts for coverage

[yash-atreya]

Motivation

Closes #8840

Saves coverage artifacts and build-info to a separate directory out/coverage and out/coverage/build-info respectively. This prevents users from mistakenly deploying unoptimized contracts, see: #8840 (comment)

Solution

Artifacts generated by forge coverage are written to out/coverage.

[klkvr]

should we just flip the second bool here?

foundry/crates/forge/bin/cmd/coverage.rs

Line 110 in 55badd4

let mut project = config.create_project(false, false)?;

coverage should not write any artifacts given that it already disables cache

[yash-atreya]

should we just flip the second bool here?

foundry/crates/forge/bin/cmd/coverage.rs

Line 110 in 55badd4

let mut project = config.create_project(false, false)?;

coverage should not write any artifacts given that it already disables cache

While not in this PR, I'd like to enable caching for coverage (backlog from: #9366). Moreover, I think that would be severely breaking for any external workflows that users have ?? cc @grandizzy @zerosnacks

[grandizzy]

should we just flip the second bool here?

foundry/crates/forge/bin/cmd/coverage.rs

Line 110 in 55badd4

let mut project = config.create_project(false, false)?;

coverage should not write any artifacts given that it already disables cache

While not in this PR, I'd like to enable caching for coverage (backlog from: #9366). Moreover, I think that would be severely breaking for any external workflows that users have ?? cc @grandizzy @zerosnacks

I think in scope of solving issue here #8840 (comment) flipping no_artifacts to true makes sense (indeed could be breaking for existing flows but better to break instead having wrong assumptions as pointed out in comment) . If we want to enable caching then IMO PR changes are OK but maybe we could also set both cached and no_artifacts to true?

[yash-atreya]

@grandizzy

I think in scope of solving issue here #8840 (comment) flipping no_artifacts to true makes sense

Makes sense. Can flip no_artifacts to true for this PR and merge.

[yash-atreya]

@grandizzy @klkvr ptal

[grandizzy]

lgtm!