fix(check): skip exported config for formatters when workspace file exists#2711
fix(check): skip exported config for formatters when workspace file exists#2711marschattha wants to merge 2 commits intomainfrom
Conversation
|
Coverage Impact - ubuntu-latest ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (1)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
There was a problem hiding this comment.
Pull request overview
This PR aligns config precedence behavior between linters and formatters by preventing exported formatter configs from being staged when an equivalent config file already exists in the workspace root.
Changes:
- Detects an existing workspace-root config file corresponding to an exported config.
- Skips symlinking exported configs into the staging area for formatters when the workspace file exists (so the workspace file should win).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
Coverage Impact - macos-15 ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (1)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
marschattha
force-pushed
the
ma/fix-formatter-exported-config-precedence
branch
from
df74cf5 to
806a384
Compare
noahd1
left a comment
noahd1
left a comment
There was a problem hiding this comment.
Not saying this is an issue per se .. but just wanting to understand:
- if you previously were using formatting with a shared config, the shared config would win.
- now, your local config file will win, and formatting may look different for you than previously
- how does a customer know which config file "won" when these situations occur? Do we save it to any of our tables / output?
Correct
Correct, if there is someone who has a shared config and repo based config that is different it can look different based on said config, and there is also the situation where a plugin has both linters and formatters, prior to this PR, formatter would be using the exported config and linter would be using the repo level config.
There is a lack of visibility there I believe since we added supported for a number of different options but haven't made it clear in one place, the debug logs can add some light to it I believe as it logs when it symlinks/copies files. |
…xists Load repository config files into the staging area before exported configs so that workspace files take precedence. The existing to.exists() guard in load_config_file_from naturally skips the exported config when the workspace file is already present, making formatter behavior consistent with linters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
marschattha
force-pushed
the
ma/fix-formatter-exported-config-precedence
branch
from
806a384 to
c5783fb
Compare
So for some unknown subset of users who have both local configs and shared configs, all their PRs may start failing with formatting errors when a new config takes precedence where it hadn't previously. I think we may need to hold off on this change for the moment until we are wide eyed about the potential impact, even if it's the right change. |
brynary
left a comment
brynary
left a comment
There was a problem hiding this comment.
Can we please start with a test that would fail without this fix?
…r exported config
Verify that workspace config files take precedence over exported
config files for both linters and formatters. The test uses distinct
issue messages ("WORKSPACE" vs "EXPORTED") so a failure directly
indicates which plugin used the wrong config.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Test plan
🤖 Generated with Claude Code