Conversation
Co-authored-by: Dmitri Khokhlov <7530150+dkhokhlov@users.noreply.github.com> Co-authored-by: Austen Stone <22425467+austenstone@users.noreply.github.com> Co-authored-by: Federico Grandi <26386270+endbug@users.noreply.github.com> Co-authored-by: Odd Stråbø <638706+oddstr13@users.noreply.github.com> Co-authored-by: Felipe Santos <29582865+felipecrs@users.noreply.github.com> Co-authored-by: Fernando Fernández <10274099+ferferga@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
oddstr13
left a comment
There was a problem hiding this comment.
Please keep commits intact.
At the very least, keep one commit per contributing person.
I am not happy with the way attribution is getting squished here.
I won't be testing this PR, as I decided it wasn't worth my time to figure out the new errors that showed up (and caused failure email notifications to be generated), and as such disabled it on my profile a while back.
|
Ideally, you would merge in the individual branches that you are cherry-picking from here, keeping all commits with their original commit info. GitHub has instructions for how to merge a commit on the command line, and these instructions can be used to merge changes even tho a particular tree/PR isn't targeted at your own repository. It is also a good idea to link back to the specific PRs that are included in a combination PR such as this one. For example, my changes exist here; lowlighter#1754 This helps maintain the context of how and why changes are made, along with who and what comments it has received. |
I wasn't pleased with squashing the attribution either, but most of the PRs to upstream either a) overlap with a different PR or b) contain multiple bug fixes (usually both). Given there wasn't much hygiene there, I didn't think anyone would mind much on that point. Clearly that was an incorrect assumption. I'll pull out your branch and try to merge it separately. Can you check #25?
I'm happy to keep original authorship, etc, but this repo is using squash merging, so the original commits will end up modified. I hope that's not a problem to you, but most PRs aren't atomic and I value bisection.
No problem! I'm in the same boat, hence the request for testers.
Oh, I have them all written down but didn't post them. Silly me 🤦🏻♂️ Added them to the description. |
|
I think my branch has decently clean commits for the different bits, and thus a perfect candidate for proper cherry-pick, just discard whichever overlap with the other PRs. I know some of my changes ended up being duplicates that I didn't see before after the fact (xz-utils for example). My branch wasn't intended as a merge-ready PR, but a "this works for me", if the repo had been active, I would've made multiple PRs (cherry-picking from my soft-fork branch – I usually name that one odd-choices as I did here 😅) My main concern is the authorship info (for git blame), and not the commit hashes – I relatively regularly do rebase and/or squashing myself. The main issue that I reacted to, is that all authors are attributed for all the changes, and there is no way to tell who did what. I've worked in codebases where a large part of the git blame is "Release x", where over half the code in the project has been changed in a single commit with no additional context, which is very far from ideal. |
As often promised, this cherry-picks a bunch of PRs from the @lowlighter repository.
I've reviewed them manually but haven't had experience with any of the listed bugs myself, so I selected what seemed to be the best patch from among the PRs. I took a look at the obviously Claude Code/OpenClaw/etc PRs as well, but did not attribute most of their authors as they didn't seem to contribute anything unique that a human hadn't already written.
AI Slop PRs left unattributed
Closes #21.
CC: @dkhokhlov @austenstone @EndBug @oddstr13 @felipecrs @ferferga
Can I get a one or two of y'all to test this branch and make sure it works?