wgtmac commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2520458581
Thank you all for review and comment! @Xuanwo @raulcd @Fokko @gaborkaszab
@Zheaoli @zhjwpku @hawkingrei
It is always difficult to start something new like this because there are a
lo
Xuanwo commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2520079242
> Sure, no worries. It's good to have people unblocked by merging this PR. I
just wanted to raise attention that later on we should be a bit more patient so
that every stakeholder can have a
gaborkaszab commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2519992836
> @gaborkaszab sorry about that. I'm sorry about that. Maybe I had a
different view about the initial PR having an initial set up were more people
can start working around other things
Fokko commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2519933972
I was following this and waiting for the discussion to dry up :) We should
ensure everyone has the time to review, considering the time zones and the
upcoming holidays. I often try to get app
raulcd commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2519915564
@gaborkaszab sorry about that. I'm sorry about that. Maybe I had a different
view about the initial PR having an initial set up were more people can start
working around other things and tha
gaborkaszab commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2519796228
Just a general comment for the future: I don't think all of the active
reviewers gave an approve to this PR (I certainly didn't). For me it seemed a
bit rushed to merge. There was even
Xuanwo merged PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3
--
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
raulcd commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2519672093
@Fokko I think this is ready to be merged so we can kick start other things.
I am unsure if there are other committers that could help with merging tasks on
the iceberg-cpp implementation :)
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1870533699
##
cmake_modules/BuildUtils.cmake:
##
@@ -0,0 +1,212 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the N
zhjwpku commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1870508697
##
cmake_modules/BuildUtils.cmake:
##
@@ -0,0 +1,212 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the
zhjwpku commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1869543514
##
cmake_modules/BuildUtils.cmake:
##
@@ -0,0 +1,212 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1869578835
##
cmake_modules/BuildUtils.cmake:
##
@@ -0,0 +1,212 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the N
hawkingrei commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2516706798
> Should we think about the Bazel project? I think it's more advanced than
cmake
In fact, there are already projects that can support both bazel and cmake
such as [abseil/abseil-c
raulcd commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1869100965
##
cmake_modules/BuildUtils.cmake:
##
@@ -0,0 +1,212 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the N
wgtmac commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2516656810
> Should we think about the Bazel project? I think it's more advanced than
cmake
That's a good question. I think Bazel is a good option if the majority of
active contributors have a g
raulcd commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2516655380
> Should we think about the Bazel project? I think it's more advanced than
cmake
We can provide CMake and if someone wants to add meson, bazel or other build
systems this can be done
Zheaoli commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2516529806
Should we think about the Bazel project? I think it's more advanced than
cmake
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to Git
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867948819
##
cmake_modules/BuildUtils.cmake:
##
@@ -0,0 +1,212 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the N
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867920857
##
src/core/icebergConfig.cmake.in:
##
@@ -0,0 +1,22 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the N
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867914158
##
cmake_modules/BuildUtils.cmake:
##
@@ -0,0 +1,212 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the N
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867907240
##
src/core/demo_table.h:
##
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTIC
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867907240
##
src/core/demo_table.h:
##
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTIC
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867907240
##
src/core/demo_table.h:
##
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTIC
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867897791
##
README.md:
##
@@ -21,6 +21,11 @@
C++ implementation of [Apache Iceberg™](https://iceberg.apache.org/).
+## Requirements
Review Comment:
Good suggestion! I'll a
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867893586
##
CMakeLists.txt:
##
@@ -0,0 +1,63 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# dis
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867887143
##
api/CMakeLists.txt:
##
@@ -0,0 +1,22 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+#
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867879414
##
CMakeLists.txt:
##
@@ -0,0 +1,63 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# dis
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867875517
##
CMakeLists.txt:
##
@@ -0,0 +1,63 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# dis
gaborkaszab commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1866085625
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
Seems great!
--
This is an automated message from the Apache Git Service.
To respond to the message, p
gaborkaszab commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1867249560
##
CMakeLists.txt:
##
@@ -0,0 +1,63 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+
wgtmac commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2513818638
I think I have addressed all comments. The `BuildUtils.cmake` is borrowed
from Apache Arrow (a comment has been added to reflect this). Let me know what
do you think. Thanks! @Fokko @gaborka
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
raulcd commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1866022415
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
what is `api` in this context? is this the public file headers to include?
--
This is an automated message
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865982169
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
Personally I'm not in favor of splitting this repo into multiple libraries,
which is an overkill. Originally I
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1866008092
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
Sounds good! It is still unclear what it will look like eventually. I'm
adding third-party libraries (arrow, a
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865972561
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
I'm not sure if I understand it correctly. Do you mean that the `api` folder
stores all public headers? Then e
gaborkaszab commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865992819
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
Ok ,what if we start with api/ core/ puffin/ and example/ and then we'll see
if there is a need for anyth
gaborkaszab commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865953000
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
In my opinion we'd need at leas api/ and core/ (I'd prefer core/ over
libiceberg just to be in line with
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1866036591
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
What about this?
```
iceberg-cpp/
├── api/
│ ├── table.h
│ └── puffin.h
├── example/
│
raulcd commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1866034166
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
ok, just read the other comment. No strong opinion on calling it `include`
or `api`, I've seen it call it both
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865575644
##
CMakeLists.txt:
##
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# dis
raulcd commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r186410
##
CMakeLists.txt:
##
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# dis
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865421713
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
Please see my comments above. I think this relat
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865415441
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
Do we need more libraries other than `libiceberg` (which is the core library
in your structure)? In my mind, `
wgtmac commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865407281
##
include/CMakeLists.txt:
##
@@ -0,0 +1,22 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
The `include` folder is for public header
gaborkaszab commented on code in PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#discussion_r1865359742
##
src/demo.cc:
##
@@ -0,0 +1,26 @@
+/*
Review Comment:
I think I'd structure the code a bit differently. How I imagined the
structure of the c++ lib is somethin
Fokko commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2509612587
Thanks for kicking this off @wgtmac 🙌
I don't have a strong opinion on this, mostly due to my lack of C++
knowledge. I do think it would be good to add a small section to the
`README.
wgtmac commented on PR #3:
URL: https://github.com/apache/iceberg-cpp/pull/3#issuecomment-2509019014
This is the first effort to make sure the project can build successfully
before providing concrete PoC implementations. Please take a look and provide
your feedback. Thanks! @Fokko @Xuanwo @
wgtmac opened a new pull request, #3:
URL: https://github.com/apache/iceberg-cpp/pull/3
Added basic CMake files for a toy iceberg library. This is a start point for
following PoC implementations. I have assumed the minimum CMake version is 3.20
and C++ version is C++20. They are subject to
49 matches
Mail list logo
