No need for apologies, Wes, I appreciate you keeping this on your radar. I've made the changes and have pushed them to the PR branch. You can begin your review when you get the chance.
--TPB On Thu, May 2, 2019 at 3:32 PM Wes McKinney <wesmck...@gmail.com> wrote: > + Parquet dev list > > Thanks Tim for working on this issue, I'm sorry I haven't been able to > leave code review yet -- I've been busy with a bunch of other things > and, since it's a large patch, I wanted to give thoughtful feedback. > > Feel free to push some more commits to that PR. I can prioritize > getting you some feedback in the next couple of working days, just let > me know when you're ready for me to review. > > On Thu, May 2, 2019 at 5:23 PM TP Boudreau <tpboudr...@gmail.com> wrote: > > > > Hello Parquet-Arrow Team, Wes, > > > > A short while ago, I submitted PR 4185 ( > > https://github.com/apache/arrow/pull/4185) to implement in the C++ > library > > the new logical annotations metadata available in the latest > parquet.thrift > > spec (https://issues.apache.org/jira/browse/PARQUET-1411). I stopped > > committing to that PR's branch about a week ago to allow the code to be > > reviewed without it being a moving target. > > > > I've since (optimistically) starting blocking out new code for ARROW-3729 > > based on my open PR (switching Arrow to read/write the new Parquet > > annotations, https://issues.apache.org/jira/browse/ARROW-3729) and while > > doing that realized that usage of the annotations classes I created in > the > > open PR might be smoother with the introduction of a few convenience > > methods. However, the most suitable names for these methods (IMO) were > > introduced for another purpose in the open PR and would need to be > > reclaimed -- overall a fairly minor, non-structural change to the PR > code. > > > > I can either add another commit to the open PR to add these convenience > > methods and rename some things (which would be my preference, provided no > > one has invested too much time yet reviewing it -- maybe you have Wes?), > or > > I can wait for the first round of reviews on that PR to see where things > > stand. > > > > How should I proceed? > > > > Thanks in advance, > > --Tim >
