atsteffen commented on code in PR #43:
URL:
https://github.com/apache/maven-build-cache-extension/pull/43#discussion_r1129557444
##########
src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java:
##########
@@ -173,11 +173,14 @@ public CacheResult findCachedBuild(
LOGGER.info("Attempting to restore project {} from build cache",
projectName);
// remote build first
- result = findCachedBuild(mojoExecutions, context);
+ if (cacheConfig.isRemoteCacheEnabled()) {
Review Comment:
1. For testing flag and xml setting combinations, a lightweight unit test
seems prudent. I went ahead and added a unit test for CacheConfigImpl. I
tested the initialization and the remote parameters only, but structured it in
a way that it should easily accommodate tests around other configurations. A
couple integration tests make sense as well, but I thought i'd wait until PR
#34 is merged to build off that approach. Tests are a great way to guide
meaningful cleanups and refactors, and I feel I need some more time and context
to do this at an integration level.
2. Remote configurations are now preconditioned on any meaningful
encompassing configuration behind the CacheConfig interface.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
