gaborkaszab commented on code in PR #3: URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865947614
########## include/CMakeLists.txt: ########## @@ -0,0 +1,22 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: I gave this a second thought: I think in the Java implementation there is a purpose of having an api/ module: Whatever is there isn't expected to change out of the Blue. Once you have anything in the spec you have to deprecate first in one of the upcoming releases and you can drop only in the following major release. I think we can follow this as well: Have an api/ folder for the headers that we will consider the part of the lib the users should interact with. In Java these are interfaces if I'm not mistaken and then the actual implementations are part of the other modules like core/ etc. For instance Table is in the api/ and is a pure interface while inherent classes like BaseTable are in core/ I find this something we should also follow. -- 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
