Skip to content

Conversation

@hadley
Copy link
Member

@hadley hadley commented Aug 14, 2025

Fixes #2217

@hadley hadley requested a review from gaborcsardi August 14, 2025 18:47
@claude
Copy link

claude bot commented Aug 14, 2025

Claude finished @hadley's task —— View job


Code Review Progress

  • Read and analyze changed files
  • Check for potential bugs or issues
  • Provide review feedback

Analysis

I've analyzed the PR changes:

R/reporter-progress.R: The main change is in the ParallelProgressReporter's end_file() method around line 482-496. When is_full() is true (max failures reached), it now:

  1. Shows all collected issues from all files before terminating
  2. Displays issues using the same pattern as the regular progress reporter

Snapshot changes: The test snapshots show that the parallel progress reporter now displays all 11 failures when hitting the max_fail limit, rather than terminating immediately.

Review

The code looks correct. The lambda function syntax \(x) x$issues$as_list() is consistent with existing usage elsewhere in the file (line 488). The change properly collects and displays all failures before terminating, which matches the PR objective.

LGTM


Copy link
Member

@gaborcsardi gaborcsardi left a comment

Choose a reason for hiding this comment

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

I think it is OK, I had two minor comments.

@hadley hadley merged commit e4b8747 into main Aug 28, 2025
17 checks passed
@hadley hadley deleted the parallel-progress-max-fail branch August 28, 2025 12:47
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.

Sometimes failure messages not shown

2 participants