build: don't depend on test_pch unless using pch#18466
Conversation
Signed-off-by: Matt Klein <mklein@lyft.com>
|
cc @keith |
Signed-off-by: Matt Klein <mklein@lyft.com>
| repository + "//test:main", | ||
| repository + "//test/test_common:test_version_linkstamp", | ||
| ], | ||
| ] + select({ |
There was a problem hiding this comment.
Sorry what do you mean? You need brackets around the select? Or inside the select?
There was a problem hiding this comment.
(I think we have brackets inside the select?)
There was a problem hiding this comment.
ah ok yea sorry that works too, i've seen it the other way in this repo so i just assumed
| "//source/extensions/common/dynamic_forward_proxy:dns_cache_manager_impl", | ||
| "//test/mocks/network:network_mocks", | ||
| "//test/mocks/runtime:runtime_mocks", | ||
| "//test/mocks/server:factory_context_mocks", |
There was a problem hiding this comment.
Did you intend for these two BUILD file changes to be in this PR?
There was a problem hiding this comment.
Yes, it's required because we no longer basically build the world for each test. I think there is an issue with how much test_pch brings in that we need to also fix.
|
You over built the first time for targets with no dependencies |
| repository + "//test:main", | ||
| repository + "//test/test_common:test_version_linkstamp", | ||
| ], | ||
| ] + select({ |
There was a problem hiding this comment.
ah ok yea sorry that works too, i've seen it the other way in this repo so i just assumed
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
|
I had to drop coverage since somehow this change effects the comment accounting. :( cc @lizan @alyssawilk |
4 participants
Risk Level: Low
Testing: Existing tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A