elharo commented on code in PR #148:
URL:
https://github.com/apache/maven-assembly-plugin/pull/148#discussion_r1224195808
##########
src/main/java/org/apache/maven/plugins/assembly/mojos/AbstractAssemblyMojo.java:
##########
@@ -430,6 +430,12 @@ public abstract class AbstractAssemblyMojo extends
AbstractMojo implements Assem
@Parameter
private String overrideGroupName;
+ /**
+ * Override of umask.
Review Comment:
Now that I see the description, does it even matter that this overrides
another value? It would be simpler to just say this sets the umask directly,
and also specify the default value used if one is not specified here. The rest
is implementation detail.
##########
src/main/java/org/apache/maven/plugins/assembly/AssemblerConfigurationSource.java:
##########
@@ -223,4 +223,9 @@ public interface AssemblerConfigurationSource {
* @return Override group name.
*/
String getOverrideGroupName();
+
+ /**
+ * @return Override umask.
Review Comment:
Consistency with existing incomplete code is not a virtue.
##########
src/main/java/org/apache/maven/plugins/assembly/AssemblerConfigurationSource.java:
##########
@@ -223,4 +223,9 @@ public interface AssemblerConfigurationSource {
* @return Override group name.
*/
String getOverrideGroupName();
+
+ /**
+ * @return Override umask.
+ */
+ Integer getOverrideUmask();
Review Comment:
You never know who your dependents are. It's easy to add a return null here,
and then you don't break anyone.
--
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]
