kriegaex commented on a change in pull request #95:
URL: https://github.com/apache/maven-shade-plugin/pull/95#discussion_r637379490
##########
File path: src/main/java/org/apache/maven/plugins/shade/DefaultShader.java
##########
@@ -694,6 +702,11 @@ private void addResource( Set<String> resources,
JarOutputStream jos, String nam
List<Relocator> relocators;
+ // Use thread-local, just in case 'map*' calls are ever done
concurrently. Make sure that the using class
+ // initialises this value according to its needs, usually setting the
value to false per file before starting
+ // relocation.
+ ThreadLocal<Boolean> wasRelocated = new ThreadLocal<>();
Review comment:
Yes, in a way it is premature optimisation. I call it defensive
programming. I know that it is single-threaded at the moment, but that could
change in the future. The funny thing is, if I would have ignored the
thread-unsafety here, nobody would have raised a red flag during the code
review. The fact that I did take some precaution, suddenly makes it bad. I
disagree with that kind of assessment. But feel free to fork this, if you think
you must. I am always happy to learn. I just think, this should be done in a
future PR, not here. I would rather have addressed my question from the TODO
comment here, but there the reaction was to do it next time. I guess, different
people just have a different sense of priorities, because it is a matter of
personal judgement.
I simply disagree with you here about "easily breakable" or "fragile". I am
not going to change it.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
