cowwoc commented on code in PR #388:
URL:
https://github.com/apache/maven-build-cache-extension/pull/388#discussion_r2406815442
##########
src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java:
##########
@@ -575,7 +575,24 @@ public String getLocalRepositoryLocation() {
public List<DirName> getAttachedOutputs() {
checkInitializedState();
final AttachedOutputs attachedOutputs =
getConfiguration().getAttachedOutputs();
- return attachedOutputs == null ? Collections.emptyList() :
attachedOutputs.getDirNames();
+ if (attachedOutputs == null) {
+ return getDefaultAttachedOutputs();
+ }
+ return attachedOutputs.getDirNames();
+ }
+
+ private List<DirName> getDefaultAttachedOutputs() {
Review Comment:
Hmm, that is problematic. To my mind, the cache should focus on correctness
first because it is *extremely* difficult to debug the kinds of bugs that the
extension introduces otherwise.
Yes, it should be possible to easily target CI, CLI and IDE use-cases but I
urge you to make the default behavior safe.
In terms of opting into behavior, what do you think of the concept of
introducing both low-level properties and high-level "profiles" for doing so?
The low-level properties would allow users to opt in/out of behavior like
timestamp restoration, while high-level "usage profiles" (CI, CLI, IDE) would
set multiple low-level properties at once. That should make it easy for the
target audience to configure it to do what they want.
--
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]
