这是indexloc提供的服务,不要输入任何密码
Skip to content

fix: respect output mode 'none' even when caching is disabled #9670

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
merged 16 commits into from
Jan 10, 2025

Conversation

anthonyshew
Copy link
Contributor

@anthonyshew anthonyshew commented Jan 3, 2025

Description

This is a fresh PR for #7900, which sat too long and got stale. I accidentally closed the PR with no commits after force-pushing so I'm opening this for simplicity's sake.

@aholland deserves all credit here for finding this issue and proposing a fix.

Testing Instructions

  • Existing CI should catch me on regressions 🙏
  • Added a test
  • Did some manual testing. Things look to be doing what we want.

@anthonyshew anthonyshew requested a review from a team as a code owner January 3, 2025 15:25
Copy link

vercel bot commented Jan 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-gatsby-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-kitchensink-blog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-svelte-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am
examples-vite-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:50am

Copy link
Member

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

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

One extremely minor suggestion, but otherwise LFG!

if self.caching_disabled || self.run_cache.writes_disabled {
log_writer.with_writer(writer);
return Ok(log_writer);
if !self.caching_disabled && !self.run_cache.writes_disabled {
Copy link
Member

Choose a reason for hiding this comment

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

Translating this for myself: "if a task has "cache": true and is run at least one cache it can write to"

This is changed from: "if task has "cache": false or cannot write to disk"


// Returning the buffer's length so callers don't think this is a failure to
// create the buffer
Ok(buf.len())
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch! This was dead code before so we were never hitting the bad return of Ok(0), thanks for fixing up my mistake.

Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
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