-
-
Notifications
You must be signed in to change notification settings - Fork 428
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: use config directory as cwd, when multiple configs present #1091
Conversation
4bc7685
to
0f68ed1
Compare
I guess the only question is that should there be an info message about this... |
Codecov Report
@@ Coverage Diff @@
## master #1091 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 647 653 +6
Branches 169 173 +4
=========================================
+ Hits 647 653 +6
Continue to review full report at Codecov.
|
Maybe something like:
|
0f68ed1
to
798219e
Compare
I rebased this to #1094 |
eebd2da
to
fc46e54
Compare
fc46e54
to
1fb4e9f
Compare
Just wanted to note that I stumbled across this issue after realizing that my task binaries weren't being resolved as expected for subdirectory configurations. In our case, we've got a root configuration that covers several directories, then a subdirectory configuration that covers that one specific subdirectory. After this change, tasks from both the subdirectory and root directory are behaving as I'd expect. |
What do you think @okonet? Should we merge this and then investigate on how to merge/run nested configurations? |
🎉 This PR is included in version 12.3.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Seems like there's some bugs or surprising behavior. The changes here seem require that there be a lint-staged config in the top level directory, even if it's not needed. This does not work, though I expected it to:
This does work:
Unfortunately, the base Doesn't work:
I tried putting in junk:
but that resulted in this, even with the subdirectory files:
I tried this:
but got
Finally I just tried to print
and that worked as expected, checking and running the subdirectory configurations. I'm not sure what the expectation is for how this should behave, but this was very surprising. Maybe it will help someone else in the future. |
@bbugh thanks for the report. I guess for now there should be at least a single config file found for staged files, but maybe that shouldn't be the case? We already have the case "no staged files matched any of the globs", so if no files match a config, that'd be the same thing. |
@MinhLoi yes, the logic for discovering configs by only checking from the staged files is wrong. We need to use |
This PR makes tasks run in the directory of the config file, when there are multiple configuration files.
In the case of only a single configuration file, tasks will still run in the current working directory, so this shouldn't really be a breaking change.
I also added some debug logging to
getConfigGroups
.What do you think, @okonet?