@simontatham All this 👆 except that "URL of a git repository + branch name" is my last choice, not first, largely for the same reason: it could disappear tomorrow, or get force-pushed/replaced between when I start reviewing it and when I eventually want to merge it. This actually happened several times with things folks were trying to get in musl, & it delayed them months or worse. Personally I always want changes submitted in a form with immutable record.
Conversation
Notices
-
Embed this notice
Rich Felker (dalias@hachyderm.io)'s status on Thursday, 06-Mar-2025 12:39:19 JST Rich Felker
- Haelwenn /элвэн/ :triskell: likes this.
-
Embed this notice
Rich Felker (dalias@hachyderm.io)'s status on Thursday, 06-Mar-2025 13:06:30 JST Rich Felker
@SoniEx2 @simontatham That prevents falsification (modulo sha1 brokenness) but doesn't preserve the version submitted.
-
Embed this notice
Genders: ♾️, 🟪⬛🟩; Soni L. (soniex2@chaos.social)'s status on Thursday, 06-Mar-2025 13:06:32 JST Genders: ♾️, 🟪⬛🟩; Soni L.
@dalias @simontatham url + commit hash?
-
Embed this notice
Simon Tatham (simontatham@hachyderm.io)'s status on Thursday, 06-Mar-2025 17:03:39 JST Simon Tatham
@dalias hmm. I think there are really two separate questions there: (a) the risk of the linked-to repo vanishing _while_ you're still reviewing and considering the contribution, and (b) the risk of it vanishing long afterward so that the long-term record goes away.
I've never considered (b) particularly important, although I'm open to being persuaded otherwise. To my way of thinking, once I've accepted a patch, the _final_ version of the patch is important to preserve, because it reflects the history of how my actual code evolved. But all the review drafts are just scaffolding – important during construction, but after you're done you can safely take them down and throw them away. I apply this equally to other people's patches sent to me, and my own patches I send to other people.
I've never yet had the problem of the repo containing the PR branch vanishing in mid-review. I would have guessed that it wouldn't be common because it's in the submitter's interest to keep it running! But I suppose I'm thinking on the timescale of weeks at most, however long it takes me to either accept or reject.
If PRs were public, then it seems more plausible you'd want to keep even the rejected ones forever. A patch I didn't think was worthy of getting into _my_ version of the code might still be useful to someone else to apply downstream of me.
-
Embed this notice
Simon Tatham (simontatham@hachyderm.io)'s status on Thursday, 06-Mar-2025 17:19:43 JST Simon Tatham
@dalias on the part of "can get force-pushed between starting to review and merging it": I guess that might mean you like to review patches by pointing a browser at the remote site?
I can see that if someone points me at a clone of my repo on Github, and I look at the patch in a browser and like it, then there's a risk that the version I 'git fetch' and merge into my repo might not the the same one.
But that's not how I do it. I 'git fetch' _before_ the review, downloading a copy of the candidate branch to a checkout on my own machine. Then I review the patch in gitk, probably compile and test it too (after at least one eyeball review pass), and if I'm happy, merge it. And all three of those steps refer to the same version of the patch, because I only ran 'git fetch' once (unless the submitter notified me to go and check back for more changes).