Feature request: Apply fixes directly in the commits

[FFY00]

Currently, the bot can push a new commit fixing the files that were modified by the hooks. This is great! However, I think the main use is for squash commit PRs, it does not work so well for the rebase or merge commit options. It would be great if the bot could run pre-commit against each commit and try to fix it up. This is naturally not possible every time, there may be conflicts, in which case the bot would just error out. We could then have bot command force trigger the new commit fix, as it is done currently.

[asottile]

sorry, not going to modify people's commits -- I think the current option is fine, if you want to rebase the commits feel free but there isn't really a safe way to force push over commits

[FFY00]

The current option already is unsafe. It will mess up the history if I haven't synced up my local history to the remote. The way you solve this is by rebasing on the remote, which is the same thing you would do to resolve the bot force-pushing over your commits. Both approaches can result in conflicts, however in the force-push if the commits differ too much, git may try to apply both commits (the old and the new one the bot pushed), but this is easily sorted out by removing the old commit from the rebase. I do not believe my proposed approach is significantly more unsafe than what currently happens, but it is more unsafe.

I think having the bot comment a quick description of what it did and what to do if you run into issues, and perhaps add an "undo" command, would be a reasonable approach to combat such issues. Note that this would still be helpful with the current approach.

Even if you don't feel comfortable enough to have it be automatic, what do you think about having it as a command?

Anyway, if you don't want to implement this, no worries. I just thought it would be worth asking as it would be useful, for me at least :stuck_out_tongue_closed_eyes:

[asottile]

The current option already is unsafe

there's no force pushing so yes it is safe. it can't eliminate or rewrite history

[FFY00]

I guess it depends on your definition of safe, if I push to my branch and then try to push new commits, the push will fail because the bot added a commit. I would not consider this "safe", but that is entirely subjective so it doesn't really matter. My argument is that the way you fix that situation is the same way you would fix out of sync history if the bot force pushes, the latter only having a slight possibility of an extra minor issue, as I described in my previous reply.

I believe this is entirely reasonable, but I don't want to argue.

What do you think about having this as a command? Which I have to explicitly request?

[FFY00]

By the way, as I hinted in my previous reply, I think a clean solution for the possible issues with both approaches would be to have a undo command.

[asottile]

nah sorry, I'm not going to implement something that would end up in an irreversible state -- it'd be pretty easy to write a github action to do what you want though

By the way, as I hinted in my previous reply, I think a clean solution for the possible issues with both approaches would be to have a undo command.

can't undo a force push!

[FFY00]

Github will show the commit hashes for every single force push on the PR timeline, you can simply git reset --hard to go back to a previous version. Force pushes can easily be undone. And if I have a different local history than the PR, I can just force push to my branch and then let the bot do its work again.

If you want an example of what I am referring to about Github showing the hashes, take for example https://github.com/pypa/build/pull/261. The timeline says

FFY00 force-pushed the FFY00:fix-integration branch from a1efd12 to 15ef965 on Mar 18

I can just git reset --hard a1efd12 to go back. But as I say previously, I would just force push my new changes and let the bot operate on the PR again anyway...

[asottile]

you can only do that if you have that commit checked out locally -- after the force push it's no longer reachable for new clones. pre-commit.ci does not keep your code except when it is performing a run

[FFY00]

Github will keep those commits, you can fetch them. You can still fetch the orphan commit from my example: https://github.com/pypa/build/commit/a1efd12f019736fe086cbaf6e97b6205019a63da

[FFY00]

(also, github will link you to the commits in the timeline, I believe this is pretty well accessible)

[asottile]

no sorry, you're wrong. that's not how git works.

$ git clone git@github.com:pypa/build
Cloning into 'build'...
remote: Enumerating objects: 1890, done.
remote: Counting objects: 100% (173/173), done.
remote: Compressing objects: 100% (84/84), done.
remote: Total 1890 (delta 91), reused 141 (delta 73), pack-reused 1717
Receiving objects: 100% (1890/1890), 428.17 KiB | 2.16 MiB/s, done.
Resolving deltas: 100% (1059/1059), done.
asottile@babibox:/tmp
$ cd build/
pre-commit installed at .git/hooks/pre-commit
$ git fetch origin a1efd12
fatal: couldn't find remote ref a1efd12

if you're a paying, hosted customer maybe I'll consider it. but I'm not going to implement an error prone, unsafe, shotgun of a feature for free.

[FFY00]

This in possible in git but disabled by default due to security concerns that don't apply here. I thought Github already allowed this, but apparently not. Sorry for not testing, I am replying on my phone, and sorry for wasting your time. Your reasoning makes sense, given that github does not support this.