phpro / grumphp

A PHP code-quality tool
MIT License
4.15k stars 433 forks source link

How do you think about a configuration to opt-in to continue the commit if fixer could fix everything automatically? #1132

Closed talkinnl closed 2 days ago

talkinnl commented 6 months ago
Q A
Version 2.5.0
Bug? no
New feature? yes
Question? yes
Documentation? no

We're using:

    fixer:
      enabled:true
      fix_by_default: true

The documentation (https://github.com/phpro/grumphp/blob/v2.x/doc/parameters.md ) says:

GrumPHP provides a way of fixing your code. However, we won't automatically commit the changes, so that you have the chance to review what has been fixed!

I actually experience that I always like all the automatic fixes. So if only automatic fixes were done, I'm just used to immediately committing again.

This is a compliment: I just trust Grumphp and the fixer. :)

So I wonder: Could I speed up my workflow by having an new opt-in configuration to continue with the commit if only automatic fixes have been applied?

veewee commented 6 months ago

Hello there,

Thanks for the compliment.

I've always been very sceptical for having this feature in Grumphp because I don't want to receive bug reports blaming us that our tool committed unwanted code to their codebase.

However, I do agree that having to recommit e.g. cs-fixer changes is a bit tedious.

For me, there are 2 ways to go here:

The way I see the configuration option, is something like:

grumphp:
    fixer:
      stage_fixed_files: true

This flag would be disabled by default. When enabled, an additional RunnerMiddleware could run a process like:

$stageFixedFilesArguments = $this->processBuilder->createArgumentsForCommand('git');
$stageFixedFilesArguments->add('add');
$stageFixedFilesArguments->addFiles($files);

$process = $this->processBuilder->buildProcess($stageFixedFilesArguments);
$process->run();

// Check result...

Alternatively, the configuration could accept context flags like: run, pre-commit, ... to only run in very specific contexts.

I think I'm opening up for having this in grumphp's core at this moment in time. However, I'm not going to write it myself. Care to give it a PR?

talkinnl commented 6 months ago

Thanks for the reply and the hints. Does sound like a little bit more work than I initially would've guessed, but I want to give it a try as soon as I find spare time. :)

veewee commented 2 days ago

Closing because of inactivity. Feel free to start out an extension to prove this functionality.