Questions about PR "namespacing"

[MichaelSims]

First things first: thanks for making git spr available! As a person who used to love working with Gerrit and is now forced to use GitHub PRs, it's a godsend.

I've ran into a couple of behaviors that were initially surprising to me as an ex-Gerrit user, and they are both related to how git spr "namespaces" PRs.

• Namespacing by GitHub UserID
  • I'm not sure what the rationale here is. As far as I can tell, the CommitID that is generated is a UUID, so further qualifying the branch by UserID shouldn't be strictly necessary unless you want to support a scenario where Bob and Alice both have separate PRs that have the same CommitID. The existing namespacing makes collaboration on a stack difficult... if I push up a stack namedspaced by my ID, I cannot easily hand that work over to another team member and have them resume the work. If they run a git spr update, they will create a brand new set of PRs. (This one has already bitten me)
• Namespacing by local branch name
  • This one was called out in this issue but I don't quite understand the rationale. Which commits are in a stack could be determined solely by which local commits are reachable by HEAD, leaving the local branch name out of it. If I have two stacks of work, I can still create two local branches, but I don't see why the local branch name should be encoded in the remote branch "key". This one is easier to work around, but it is still surprising coming from Gerrit, which does not care what your local branches are called.
  • Why does this matter? Again, it makes collaboration more complicated. If I'm working with another team member on a stack, they are required to create a local branch with the exact same name as the one I chose, or when they run an update, they will create "duplicate" PRs. Even outside of collaboration this can bite you; say you want to push your current stack to a different ref name in the remote and you git checkout -b some-new-name && git push. You have to take care to switch back to the original branch before running git spr update or else you risk creating a bunch of PRs that you have to go manually delete.

I understand different people have different workflows and mine isn't necessarily the "correct" one, but I feel like both the user ID and the local branch namespacing could be dropped without any loss of functionality. Any thoughts? Thanks in advance...

[ejoffe]

I had the exact same experience when I moved from Gerrit to GitHub, and that's why I wrote spr, glad it is useful for you as well.
spr works by matching the commits in your local stack with pull requests on github, the local branch name is used to filter all your pull requests on github down to the ones that match your stack (by matching on branch name).
This logic could be changed to go through all the pull requests and only match with commit-ids that are present in the stack. The only thing I can think of that will be lost using this approach is the case where you drop a commit from your stack. In the current behavior the github pull request will be automatically closed, if we only match on commit-id, we'll have no way to tell that a stale pull request for which a commit doesn't exist anymore needs to be closed. Not sure if that is a must have feature and the benefits out weight the cons.
Regarding user id, I looked at the code again now, and I don't think this is really used and can be safely removed from the namespace.

[MichaelSims]

Thank you! Yeah, the automatic closing is a good point.

I've got a branch in my fork where I started the work of removing the UserID... and it seemed simple, but then I grew concerned that perhaps the code was determining which PRs were "owned" by the current user by grabbing them all and then filtering by UserID, so I was worried I had introduced some bugs by not thinking it through thoroughly enough.

Anyway, I plan to revisit when I can because good support for collaborative workflows is essential for my team to adopt this, and I'm highly motivated to persuade them to adopt it. I'm guessing you'll want to keep this tool closer to your original vision but if you're interested in contributions for this, let me know.

Thanks again for taking the time to answer, and thanks for git spr!!

[ejoffe]

I also like the idea of making spr more collaborative friendly.
At Inigo with my current team, we usually don't amend each others commits. Most times we try to keep commits small and keep on merging. But situations have risen that required multiple users to work on a single commit, and I agree with you that the current workflow doesn't make that easy and straight forward. I'll be happy to discuss further enhancements and ideas if you are up for it.

[MichaelSims]

My team tries to adopt a mobbing/pairing approach, and we use this heavily for mentoring and knowledge sharing. So depending on who's "driving" it's nice to be able to amend a commit/PR that someone else pushed.

I'll think on the whole "local branch" namespacing/closing commits and if I come up with any clever ideas, I will share. Ultimately though if the cost of removing this namespacing is that some PRs occasionally get orphaned, I think I may accept that in order to have the flexibility of not having to care what my local branches are called and which ones I have currently checked out...

[ejoffe]

git spr sync (undocumented) might come in useful for your use case. although we haven't used it much, so consider it a beta feature.