[GitHub] [iceberg] kbendick edited a comment on pull request #3879: Core: Remove duplicate Conf() method.

[GitBox <gi...@apache.org>]

kbendick edited a comment on pull request #3879:
URL: https://github.com/apache/iceberg/pull/3879#issuecomment-1011872346


   Hi @hililiwei. Thanks for looking into this.
   
   My thoughts are:
   1) We tend to prefer method calls that don't use `get` in the name (or `set`). Either some other verb is used to be more informative (examples like `find`, etc) or we just don't use `get` since it likely provides little meaning if there isn't a more specific verb to use. That's probably why we have the two methods. So if we do remove one, it would be more in line with the project style to keep `.conf()` and remove `.getConf()`.
   2) This is a rather old API. My opinion is we can't necessarily be sure if people are depending on this API or not. I'm interested in hearing other people's opinions on this, but I think it might be safer to keep both and mark `getConf` (or at least one of them) as deprecated.
   
   Please don't spend the time updating the PR to use `conf()` instead of `getConf()` unless somebody else chimes in though so you don't make unnecessary updates 😄 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org