bitoffdev commented on a change in pull request #1959: URL: https://github.com/apache/hbase/pull/1959#discussion_r444922986
########## File path: hbase-shell/src/main/ruby/hbase/admin.rb ########## @@ -701,40 +703,40 @@ def alter(table_name_str, wait = true, *args) # Delete column family if method == 'delete' raise(ArgumentError, 'NAME parameter missing for delete method') unless name - htd.removeFamily(name.to_java_bytes) + tdb.removeColumnFamily(name.to_java_bytes) hasTableUpdate = true # Unset table attributes elsif method == 'table_att_unset' raise(ArgumentError, 'NAME parameter missing for table_att_unset method') unless name if name.is_a?(Array) name.each do |key| - if htd.getValue(key).nil? + if tdb.build.getValue(key).nil? raise ArgumentError, "Could not find attribute: #{key}" end - htd.remove(key) + tdb.setValue(key, nil) Review comment: Unfortunately, the current overloads for `TableDescriptorBuilder.removeValue` take either `Bytes`, or `byte[]`, whereas `setValue` has an overload for a String argument. **Unless there is an objection, I will go ahead and add the `removeValue(final String key)` overload**, which would be consistent with the `setValue` overloads, and may be helpful for other developers in the future as well. The other alternative (converting the key to bytes) seems like it would be more effort than just using `setValue` as is. ---------------------------------------------------------------- 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: us...@infra.apache.org