rdblue commented on a change in pull request #4307:
URL: https://github.com/apache/iceberg/pull/4307#discussion_r825474024
##########
File path:
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/actions/BaseDeleteOrphanFilesSparkAction.java
##########
@@ -107,10 +110,12 @@ public void accept(String file) {
public BaseDeleteOrphanFilesSparkAction(SparkSession spark, Table table) {
super(spark);
- this.hadoopConf = new
SerializableConfiguration(spark.sessionState().newHadoopConf());
- this.partitionDiscoveryParallelism =
spark.sessionState().conf().parallelPartitionDiscoveryParallelism();
+ hadoopConf = new
SerializableConfiguration(spark.sessionState().newHadoopConf());
+ partitionDiscoveryParallelism =
spark.sessionState().conf().parallelPartitionDiscoveryParallelism();
this.table = table;
- this.location = table.location();
+ includeHiddenPaths = PropertyUtil.propertyAsBoolean(table.properties(),
INCLUDE_HIDDEN_PATHS,
+ INCLUDE_HIDDEN_PATHS_DEFAULT);
+ location = table.location();
Review comment:
We use the prefix `this.` so that it is easy to see when a statement
sets an instance field rather than a local variable.
Also, even if we want to make a change, we tend to prefer leaving code that
doesn't need to change alone so that we avoid commit conflicts.
Can you revert all of the changes to remove `this.`?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
