Skip to content

lib: more asyncification#9094

Open
thoughtpolice wants to merge 2 commits intomainfrom
aseipp/lib-make-treestate-check-out-nzxkkzmxzwor
Open

lib: more asyncification#9094
thoughtpolice wants to merge 2 commits intomainfrom
aseipp/lib-make-treestate-check-out-nzxkkzmxzwor

Conversation

@thoughtpolice
Copy link
Member

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by an LLM.
  • For any prose generated by an LLM, I have proof-read and copy-edited with
    an eye towards deleting anything that is irrelevant, clarifying anything
    that is confusing, and adding details that are relevant. This includes,
    for example, commit descriptions, PR descriptions, and code comments.

Assisted-by: Opus 4.6 via Claude Code
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Assisted-by: Opus 4.6 via Claude Code
Signed-off-by: Austin Seipp <aseipp@pobox.com>
@thoughtpolice thoughtpolice requested a review from a team as a code owner March 12, 2026 20:21
commit_builder.set_author(new_author);
}
commit_builder
})
Copy link
Member

Choose a reason for hiding this comment

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

Does it work to still call .map() but with an async closure and then wrap the whole think in try_join_all()? We've done that in lots of other places. It would mean that the commits are written concurrently if the backend supports it.

Copy link
Contributor

@ilyagr ilyagr Mar 12, 2026

Choose a reason for hiding this comment

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

(Non-actionable, also mostly just wrong) I only glanced at this, but it sounds a bit similar to what I'm doing in #9087 maybe: create a stream of futures, and then convert it into a stream of outputs of these futures via .buffered.

I am not sure I whole-heatedly recommend it at this point, it does seem to add complexity.

Update: I think I misunderstood what Martin said as I started writing this. I'll leave it here, I think the analogy is interesting, but it's less actionable than Martin's comment.

Update 2: The approach of #9087 is not useful here, since you don't need streaming. You need to wait to get all the values before you can do anything with them.

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