The original discussion can be find on discord thread

See #393 as an example.

Currently a PR is a minimal reviewable object and there isn't any bigger (or smaller) object than it.

With non-stacked PR, we can review the entire stack of changes but not individuals. With stacked PR, we can review individual changes as PRs but not the whole stack (submitted feature.)

Due to this limitation, it's pretty common to explain or request a review for entire stack in discord.

Current stacked PRs are more similar to GitHub's commit review where you can review in context of specific commit. In Graphite style stacked PR workflow, multiple PRs can stack on top of same PR constructing a tree of PR stacks.

So why that happened?

Because we are basically trying to atprotate the git commit history as list of patches. Commits are self-signed and linear. The commit history itself is already a stack (or a tree which allows sharing same parent commit). The commit-id and change-id are technically some form of decentralized ID. We don't need to make 1:1 clone of git commits

A PR is a submittable object pointing specific commit-hash.

We don't need a record to represent individual commits/changes/stacks. They are already decentralized as git commits. AppView might cache them for easy reference, but we don't need to decentralize them again.

Although commit-id is not part of atproto, we can still use it as a soft-link between git-objects and atproto records. Similar to how blob-ids link between atproto records and blobs. This is why I made git-objects proposal redefining Knot as an external blob store for git objects.

There wasn't any issue on current design, but if we start sharing stacks (AAA from Fig 1-1), the evolog of a stack is not linear anymore. See Fig 1-2. Change BBB diverged to BBB' and BBB'' when PR#124 is resubmitted.

The concept of rounds (linear evolution log over a PR) can only exist for actual PR HEADs. When a user resubmits a PR, they only update the PR's HEAD and not update every individual commits included in that PR (that is how we handle PR resubmits; resubmitting every PRs included in same stack.)

My guess over current design choice is to handle resubmitted stack only with history change in future. When interdiff is empty, we want to preserve the previous discussions. I'll address this problem later in this post.

The author might resubmit with messages like "Hey, I addressed your review. plz review it again", or provide a new context over updated commit history. This message will serve similar role to 0th patch message in Mailing List Workflow.

I've considered making this as a separate dependent proposal, but considering it's pretty small, I think it's fine to include here.

Although we can submit a stacked PR and track the evolution using change-id and commit-id, change review comments should point the target 'change'. Well, as I said, the commit-id is already a decentralized identifier. Nothing is stopping us from using it as a reference to the change.

So feature review comment will reference the specific PR with at-uri, and change review comment will reference the specific commit with commit-id.

See the summary below for detailed lexicons.

See Fig 1-3 and Fig 1-4. BBB' has been diverged to BBB'' for PR#124/1 and BBB''' for PR#123/2. This is really rare case and shouldn't even happen imo, but theoretically possible. So AppView should be able to handle this kind of evolog.

For PR#123's perspective, BBB'' is useless. Therefore, page /pull/123/change/BBB should ignore the BBB''. In other words, Change review page should ignore evolog from other PR.

With shared stacks design, we can have near identical review flow to mailing list workflow.

In Fig 1-2, change CCC and DDD remained unchanged but their commit hash changed. We don't want to loose previous unresolved discussions due to history changes. So I'm introducing review-bubble-up behavior for AppView.

The AppView will basically 'bubble up' the old reviews from change's evolog if the interdiff is empty.

PR only holds a HEAD of a submission (no atproto representation for commit history)

User can resubmit a PR to update the HEAD

User can comment to specific PR submission(round), same as before.

User can comment to specific change inside the submission by referencing with commit-id instead of PR's at-uri.

sh.tangled.repo.pull represents mergeable object like PR #123 It holds metadata like target, source, title and body but does not represent the exact PR content

sh.tangled.repo.pull.submit represents each PR submissions. It is atprotated PR round holding the actual PR content which is the HEAD commit hash. Actual commit is stored in author's knot (see git-objects proposal.)

sh.tangled.repo.pull.comment represents a feature-review comment. It targets to specific PR round with at-uri

sh.tangled.repo.pull.change.comment represents a change-review comment. It targets to specific change with commit hash and might include pull.submit at-uri as a context.

We might want to merge pull.comment and pull.change.comment lexicons but I'm separating them here for clarity.

Same as current non-stacked PRs. Individual review thread for each submissions. We might not collapse those threads or at least the 0th round to show entire review history.

closer to current stacked-PRs where maintainers can review each commits separately. Round number follows the resubmission of parent PR.