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

Added support for workingDirectories to the 'lint whole folder' task #1861

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Tiberriver256
Copy link

Fixes: #633

@dbaeumer
Copy link
Member

Nice work. Wasn't aware the pushd & popd is available in all command shells.

@dbaeumer
Copy link
Member

But what I found out now is that && is not available under PowerShell so the task will fail if PowerShell is used as a command processor.

@dbaeumer
Copy link
Member

And && usually has the semantic that the second command will not run if the first fails. IMO this will stop the shell execution on the first working directory that has an eslint error since eslint returns a exit code different than 0 in that case.

@dbaeumer
Copy link
Member

@Tiberriver256 any thoughts on the above comments.

@Tiberriver256
Copy link
Author

But what I found out now is that && is not available under PowerShell so the task will fail if PowerShell is used as a command processor.

Yes :( sadly older versions of PowerShell do not have this capability. Newer versions of PowerShell core do have support for it. We could look into using ; instead but I'm not sure if that's supported in cmd.exe. It should work in Bash & PowerShell and would give us the behavior of running all the commands regardless of whether or not one failed.

And && usually has the semantic that the second command will not run if the first fails. IMO this will stop the shell execution on the first working directory that has an eslint error since eslint returns a exit code different than 0 in that case.

This is correct. Was a royal pain when I was testing it out locally to make sure I had run npm install in all my folders before it would run right.

@Tiberriver256 any thoughts on the above comments.

Overall... feels flakey... I kinda want instead of one mega task, one task per folder. Instead of seeing this:

  • eslint whole folder

I would see:

  • eslint whole ./project1 folder
  • eslint whole ./project2 folder
  • eslint whole ./project3 folder

What do you think?

@dbaeumer
Copy link
Member

I will talk to the task developer in VS Code to see what we can do here.

@dbaeumer
Copy link
Member

Here is an idea you could experiment with:

Let me know how it goes

@Tiberriver256
Copy link
Author

Here is an idea you could experiment with:

Let me know how it goes

Nice! I'll give it a go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint whole folder task ignores workingDirectories setting
2 participants