Skip to content

Conversation

@yhakbar
Copy link
Collaborator

@yhakbar yhakbar commented Oct 27, 2025

Description

Refactors find & list so that they build up the string they're going to write before writing.

Ensures write operation is atomic, and prevents unnecessary syscalls.

TODOs

Read the Gruntwork contribution guidelines.

  • I authored this code entirely myself
  • I am submitting code based on open source software (e.g. MIT, MPL-2.0, Apache)]
  • I am adding or upgrading a dependency or adapted code and confirm it has a compatible open source license
  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

Summary by CodeRabbit

Release Notes

  • Refactor
    • Enhanced output rendering efficiency in the find and list commands through optimized buffer handling.

@vercel
Copy link

vercel bot commented Oct 27, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
terragrunt-docs Ready Ready Preview Comment Nov 5, 2025 1:39pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 27, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Two CLI command files are refactored to buffer output before writing. The find.go command's outputText function and list.go command's rendering functions (renderLong, renderTabular, renderDot) now use strings.Builder to accumulate formatted output in memory and write once, replacing per-item direct writes.

Changes

Cohort / File(s) Summary
CLI output buffering optimization
cli/commands/find/find.go, cli/commands/list/list.go
Both files refactored to buffer text output using strings.Builder before writing. find.go's outputText accumulates formatted lines. list.go's render functions (renderLong, renderTabular, renderDot) buffer output and write once. In list.go, function renderLongHeadings (error-returning) is replaced with buildLongHeadings (string-returning); error handling consolidated to final write.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify error handling is correctly consolidated to final buffer write operations
  • Confirm buildLongHeadings function properly replaces renderLongHeadings behavior and integrates with renderLong
  • Check that all output content is correctly accumulated in buffers (no formatting or data loss during transition from streaming to buffered writes)

Suggested reviewers

  • denis256
  • levkohimins
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/refactor-find-list-for-single-write

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29825b8 and 5c9415c.

📒 Files selected for processing (2)
  • cli/commands/find/find.go (2 hunks)
  • cli/commands/list/list.go (4 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@yhakbar yhakbar force-pushed the chore/refactor-find-list-for-single-write branch 2 times, most recently from 2649355 to 5d7bce5 Compare October 27, 2025 15:28
@yhakbar yhakbar force-pushed the feat/moving-dag-graph-to-list branch 2 times, most recently from 73d0999 to 4fff8ac Compare October 27, 2025 21:44
@yhakbar yhakbar force-pushed the chore/refactor-find-list-for-single-write branch from 5d7bce5 to d0fe23d Compare October 27, 2025 21:50
@yhakbar yhakbar force-pushed the feat/moving-dag-graph-to-list branch 4 times, most recently from e4ca449 to d2e56e6 Compare October 30, 2025 16:42
@yhakbar yhakbar force-pushed the chore/refactor-find-list-for-single-write branch from d0fe23d to a7b88bd Compare October 30, 2025 16:49
@yhakbar yhakbar marked this pull request as ready for review October 30, 2025 20:24
@yhakbar yhakbar requested a review from denis256 as a code owner October 30, 2025 20:24
@yhakbar yhakbar force-pushed the feat/moving-dag-graph-to-list branch from d2e56e6 to 45dab5e Compare November 4, 2025 22:24
@yhakbar yhakbar force-pushed the chore/refactor-find-list-for-single-write branch from a7b88bd to e7e9374 Compare November 4, 2025 22:26
@yhakbar yhakbar force-pushed the feat/moving-dag-graph-to-list branch from 45dab5e to e919a0a Compare November 4, 2025 23:35
@yhakbar yhakbar force-pushed the chore/refactor-find-list-for-single-write branch from e7e9374 to 354ce39 Compare November 4, 2025 23:37
denis256
denis256 previously approved these changes Nov 5, 2025
Base automatically changed from feat/moving-dag-graph-to-list to main November 5, 2025 13:37
@yhakbar yhakbar dismissed denis256’s stale review November 5, 2025 13:37

The base branch was changed.

@yhakbar yhakbar force-pushed the chore/refactor-find-list-for-single-write branch from 354ce39 to 5c9415c Compare November 5, 2025 13:38
@yhakbar yhakbar merged commit b4c306d into main Nov 5, 2025
12 of 13 checks passed
@yhakbar yhakbar deleted the chore/refactor-find-list-for-single-write branch November 5, 2025 13:40
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.

3 participants