pvary commented on PR #12996: URL: https://github.com/apache/iceberg/pull/12996#issuecomment-2866954512
> > > > Thanks for the review @gyfora! I think it makes sense to rename the API-facing fields / getters / setters to avoid confusion for users. > > > > > > > > > I am not yet aware of all the conventions here, @pvary maybe you could chime in related to the naming and then I will learn once and for all :D > > > > > > It might be strange for new developers, but we always omit `get`, `set`, `is` from the method names. Here is the guide: https://iceberg.apache.org/contribute/#iceberg-code-contribution-guidelines The only exceptions are the overrides for external APIs > > In general I get the idea, but my particular concern was related to `upgradeMode` the convention clearly doesn't work well with a name like this as it's immediately confusing when you have other `xxMode` fields that are enums etc. I assume this is `upsertMode`? While I understand your concern, the IcebergSink contains `upsertMode`, and this convention is used throughout the Flink code, so I would stick to it. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
