Skip to content

feat: add request changes review action#1

Open
IliasAlmerekov wants to merge 4 commits intodaulet:mainfrom
IliasAlmerekov:feat/request-changes
Open

feat: add request changes review action#1
IliasAlmerekov wants to merge 4 commits intodaulet:mainfrom
IliasAlmerekov:feat/request-changes

Conversation

@IliasAlmerekov
Copy link
Copy Markdown

Adds ability to request changes on a PR from the TUI using Shift + R in detail view.

Comment thread src/tui.rs Outdated
Comment thread src/tui.rs Outdated
if self.list_state.selected() == Some(idx) {
AsyncResult::Diff(epoch, diff, delta_output, delta_too_large) => {
// Only update if this result belongs to the current detail session
if epoch == self.detail_epoch {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Using only detail_epoch to accept async results can still attach stale data to the wrong PR after a refresh. Refresh replaces self.prs/selection without bumping epoch, so in-flight results from the prior PR can pass epoch == self.detail_epoch and overwrite diff_cache/comments_cache/checks_cache for the newly selected PR. Consider validating both epoch and PR identity (e.g., repo+number), and/or increment epoch when refresh swaps the list while detail view is active.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You are right, thanks for catching this, and sorry I missed it initially. I fixed it by validating detail async results with both epoch and PR identity, and by bumping/resetting the detail session when refresh lands in detail view.

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