Skip to content

fix: cherry-pick from forks, finally#24

Open
lishaduck wants to merge 4 commits intomasterfrom
fixes
Open

fix: cherry-pick from forks, finally#24
lishaduck wants to merge 4 commits intomasterfrom
fixes

Conversation

@lishaduck
Copy link
Member

@lishaduck lishaduck commented Mar 19, 2026

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?

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>
@github-actions

This comment has been minimized.

Copy link

@oddstr13 oddstr13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@oddstr13
Copy link

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.

@lishaduck
Copy link
Member Author

I am not happy with the way attribution is getting squished here.

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?

Please keep commits intact.

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.
I'm also personally far more comfortable rebasing than merging There weren't any conflicts after some finagling, so I just went ahead and merged; it'll get squashed in the end.

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.

No problem! I'm in the same boat, hence the request for testers.

It is also a good idea to link back to the specific PRs that are included in a combination PR such as this one.

Oh, I have them all written down but didn't post them. Silly me 🤦🏻‍♂️ Added them to the description.

lishaduck added a commit that referenced this pull request Mar 21, 2026
lishaduck added a commit that referenced this pull request Mar 21, 2026
@oddstr13
Copy link

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.
PR/commit references in commit messages is sufficient for when squashing things down, but in general at lest one commit per author would be preferable, where at all possible.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants