Replies: 3 comments
-
|
Sorry, I probably should have played around with it a bit more before asking 😅 Using But I'm also happy to hear of any insight on stabilising comments across clients – our current line of thinking is that we can communicate which algorithm and configuration options are used when providing the review comments so that clients are required to render the diff the same way :) |
Beta Was this translation helpful? Give feedback.
-
|
The goal of the "patch id" is such that it only looks at the contextual changes in a diff, and removes the line numbers. So it's really about a stable id for a change across branches. Imagine that you make a dozen commits, and I'm only interested in one in the middle. I could apply that one commit on top of my branch, and - even if some other things had changed in the file and the hunks had moved around a little bit, we may get the same patch id. However, this assumes that we're using a similar diff mechanism. If you were using two different algorithms to generate the diff, then I think that you would get different patch IDs. Here's a change in one repository: And the same change cherry-picked into another repository, that's had some lines up top removed: (Note that it starts on context line 12 in the second repository, and 14 in the first repository.) These two do have stable patch IDs, however: But I think that it is dependent upon the algorithm; if you used As for your bigger question... that's an interesting one. Just to make sure that I understand - you are trying to determine if two diffs are equivalent, and the inputs are not necessary equal. (Sort of like the patch-id problem above; which would be effective if you could guarantee that a diff was algorithmically identical.) I'm not sure that I have a great idea here, but given that a patch-id is best effort already, I wonder how lossy it Is when you factor in the algorithms? (How many people really use different algorithms and how much does it actually affect the resulting diff? That would be an interesting experiment.) But I think that unless you're able to communicate more information (about the preimage or postimage) that this may be a challenge. 🤔 |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the detailed description! That makes sense 😊
Ya, I was wondering had I missed some cool feature where you could store some new object I hadn't heard of that perfectly represented the patch without knowing the diff information at all – alas, a guy can dream 🤤 I suppose I would phrase the bigger question, "what's the minimal information required for varying clients to faithfully represent comments on a diff?" So it seems like we need to communicate what the algorithm is as well as where that comment was made in the diff, i.e. hunk and line range. I was wondering could it be algorithm agnostic, but it seems not :) I did have another idea where if you communicate the exact diff line information alongside the comment, then any client could render that exactly – but you don't get the niceness of overlaying the comment over a full view of the diff. Anyway, you don't need to worry about that – that ones my problem 😄 |
Beta Was this translation helpful? Give feedback.
-
Hey folks :)
I'm trying to understand the diff mechanisms a bit better to sketch out a way to improve code reviews in Radicle. I came across
git_diff_patchidand saw the documentation:This seems interesting because we're looking to be able to comment on diffs and would like these comments to be stable across clients – for example a client uses a different algorithm to generate the diff.
Is this
patchidsimply for doing comparisons or does it actually create an object that can then reproduce the patch in a stable way? I'm not seeing any constructor for apatchfrom anoid, but maybe I'm not looking in the right place?Appreciate any responses 😊
Beta Was this translation helpful? Give feedback.
All reactions