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
validate compiler options in multicompiler mode #15460
Conversation
For maintainers only:
|
f24b596
to
ab63834
Compare
ab63834
to
076bc4a
Compare
@alexander-akait what do you think about this PR? should we somehow define unique name in this case? |
@vankop I think validation of the |
4f0f92c
to
e39095e
Compare
a46748e
to
832434e
Compare
@alexander-akait fixed. As I understand cache does not work correctly with same |
832434e
to
4eec173
Compare
4eec173
to
5203fbf
Compare
test/configCases/cache-filesystem/multicompiler-mode-cache-warning/warnings.js
Outdated
Show resolved
Hide resolved
4a0b4f9
to
24f8a3a
Compare
fixed @alexander-akait |
lib/config/defaults.js
Outdated
const cacheNames = new Set(); | ||
return (cache, i) => { | ||
if (!cache) return cache; | ||
if (cache === true) return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a test where we have two true
values and so don't have any problems
@vankop I made some improvements of your idea
|
Also always output warnings when you explicitly set the same cache names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to merge and merge if it is ok
We need fix our tests and do not run cache test for basic test, I will tr to fix it tomorrow or you can do it, thank you |
@vankop Thanks for your update. I labeled the Pull Request so reviewers will review it again. @alexander-akait Please review the new changes. |
What kind of change does this PR introduce?
feature
closes #14843
Did you add tests for your changes?
yes
Does this PR introduce a breaking change?
no
What needs to be documented once your changes are merged?
nothing