gaborkaszab commented on code in PR #4: URL: https://github.com/apache/iceberg-cpp/pull/4#discussion_r1875912691
########## .clang-format: ########## @@ -0,0 +1,22 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +--- +BasedOnStyle: Google +ColumnLimit: 90 Review Comment: "90 for now and do some change when we find it's not proper later." Changing this later seems painful for me. In case we, for any reason decide to increase this to 100 later, we gonna have to do alignments in the code for this. That would kind of mess up the usage of "git blame" because many lines will show the refactor and not the actual change on that line. My point here is that we have to be reasonable from the beginning and not just rush forward by copy-pasting stuff from other projects. As Fokko mentioned, the Java library uses 100 and the python uses even more, and he didn't find any issues with that. Im pretty sure it would work for the C++ project too. -- 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
