Skip to content
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

Fix SyntaxCheck launch with windows on large project #437

Conversation

Jibbarth
Copy link
Collaborator

@Jibbarth Jibbarth commented Sep 19, 2020

Q A
Bug fix? yes (on dev-master)
New feature? no
Fixed tickets #...

While @ekvedaras was testing parallelization on windows, a new bug came out (cf #414 (comment))

When we are launching SyntaxCheck on a large project with Windows, passing all files to inspect as arg may create a too long command for this system.

image

Here, I refact how we launch it. Instead passing all files as arg, we now excludes directory or files from configuration, and pass the current analysed path.

image

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@Jibbarth Jibbarth added the bug Something isn't working label Sep 19, 2020
@Jibbarth Jibbarth force-pushed the fix/syntax-check-on-large-project-with-windows branch from e8ad266 to 3971686 Compare September 19, 2020 13:01
Copy link
Collaborator

@olivernybroe olivernybroe left a comment

Choose a reason for hiding this comment

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

LGTM

Can't really validate it by running it, as I don't use windows :(
But I don't see any issues with this.

@Jibbarth
Copy link
Collaborator Author

@olivernybroe thanks for review ! I had to install a whole php ecosystem on a Windows to test 😅

@olivernybroe
Copy link
Collaborator

@Jibbarth Thats a whole other level of dedication 😲

But just go ahead with the merge 👍

@Jibbarth Jibbarth merged commit 6790309 into nunomaduro:master Sep 21, 2020
@Jibbarth Jibbarth deleted the fix/syntax-check-on-large-project-with-windows branch September 21, 2020 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants