You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/01/13 07:34:14 UTC
[GitHub] [iceberg] kbendick commented on pull request #3879: Core: Remove duplicate Conf() method.
kbendick commented 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`. Either some other verb is used to be more informative (examples like `find`, etc) or we just don't use `get` since it has very little meaning. 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