Mark console commands as final classes #944
Conversation
goetas
force-pushed
the
mark-commands-as-final
branch
2 times, most recently
from
47cbbc3 to
19780b9
Compare
|
I really don't understand why scrutinizer started to complain lately |
|
Do you mean appveyor? |
| { | ||
| proc_open($editorCommand . ' ' . escapeshellarg($path), [], $pipes); | ||
| $process = new Process([$editorCommand, $path]); | ||
| $process->setTty(true); |
There was a problem hiding this comment.
I guess this must be why Appveyor is failing
There was a problem hiding this comment.
Do you know if the feature of opening an editor works/worked at all on windows systems?
(or was simply never tested...)
If so, can we simply skip the test?
There was a problem hiding this comment.
No. (no). Probably, I don't know why an editor is involved here TBH.
There was a problem hiding this comment.
What about dropping the possibility of opening an editor from doctrine migrations? I think that is out of scope for such library...
There was a problem hiding this comment.
Had no idea this was a thing… what is it used for?
There was a problem hiding this comment.
Oh ok… well I know I wouldn't use it, because I don't like having many instances of vim running for the same project. In favor of dropping that feature.
There was a problem hiding this comment.
@alcaeus what do you think?
There was a problem hiding this comment.
I've never used it either. If it's causing problems, drop it. If it's easier to skip something on Windows, do that. I don't have any preference here ;)
There was a problem hiding this comment.
I will drop it then, as it will allow us to not depend on the symfony/process component.
did not realize was appveyor... in the last PR scritinizer complained about something... but it stopped now... |
goetas
force-pushed
the
mark-commands-as-final
branch
from
5228148 to
33ea68a
Compare
|
I think that this is ready. (still do not know what causes scrutinizer to fail) |
Summary
Mark console commands as final to avoid extension. Console commands are not meant for that propuse.