-
Notifications
You must be signed in to change notification settings - Fork 443
Improve SF 8.0 support #1199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve SF 8.0 support #1199
Conversation
78fef4f to
f202e47
Compare
f202e47 to
a38d748
Compare
|
@arnedesmedt Can you validate this works for you? Composer: |
|
Hmm, I'm now getting: Is it because of the tagged iterator? that changed? => edit: don't think so. Probably the loader isn't loading all services now. So the commands are also not tagged as commands... |
|
@arnedesmedt It seems that the changes in |
|
Yes, It works now. I only now get a deprecation error from gitonomy/gitlib: Since I also needed to update that package. |
Is this something that gets triggered from within this package's code or rather something internal in gitlob? |
|
It's triggered here: https://github.com/phpro/grumphp/blob/v2.x/src/Git/GitRepository.php#L69 But it depends on the content of your git diff if it's triggered yes or no. Let me check this further. |
|
I see, that is when you use git diff directly as stdin (which happens during commit inside the commit hook) We might need to add the
Strange that I cant seem to get that error here though. |
|
That worked, but you will need to change some other data processing too. I'm getting for example the following output: So I think grumphp is now detecting file names from the git diff that aren't files at all. But just data from the git diff, that is now in another format then it was first. |
|
@arnedesmedt maybe we should just release this version first and then focus on the gitlib? I'm trying to reproduce using |
|
yes go ahead |
|
@veewee I think, I can already see the problem, why you can't reproduce. So if they release a new version, you would be able to reproduce, I guess. |
|
Ah ok. Since we dont really need the raw data, I'm wondering if the deprecation warning should be there. |
Follow-up on #1194