kbendick commented on a change in pull request #1491:
URL: https://github.com/apache/iceberg/pull/1491#discussion_r495518748
##########
File path: api/src/main/java/org/apache/iceberg/types/PruneColumns.java
##########
@@ -22,12 +22,14 @@
import java.util.List;
import java.util.Set;
import org.apache.iceberg.Schema;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
class PruneColumns extends TypeUtil.SchemaVisitor<Type> {
private final Set<Integer> selected;
PruneColumns(Set<Integer> selected) {
+ Preconditions.checkNotNull(selected, "Selected cannot be null");
Review comment:
I agree with you on this. I was following what I thought was somewhat of
a convention - what I should really refer to as something I've seen previously
in the code base - of using the argument name directly. However, I also just
today saw some instances that are definitely much more explicit.
I will update to be `Selected field ids` as its inline with our definitions
of `Select` and `Project` as well as it still contains the argument name in it.
Please correct me if I'm incorrect about `select` vs `project`. @rdblue
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
