Hi Siddharth, I agree that the community should have time to properly review and approve of changes. I definitely try to keep you in the loop of any changes going on in the Java side and want you to have time to assess possible impact downstream. If a specific patch went in that we need to revert and take a closer look at, or a current PR that you think should have more discussion, that's fine with me - it's not too late to voice your opinion. I think it's also ok to make a request in the PR to hold off for some amount of time if needed. Like others, I am also used to having discussions within PRs for most fixes and minor design changes. I find that usually leads to a better discussion and quicker resolution, but I agree that for larger things starting with a doc can be a good idea. If you are referring to ARROW-1710/ARROW-1866, I thought these were part of the discussed plan under ARROW-1463, but I also brought up the question in the PR to clarify and discuss further. If you had other thoughts or concerns, please let us know.
Thanks, Bryan On Wed, Nov 29, 2017 at 12:45 PM, Wes McKinney <wesmck...@gmail.com> wrote: > I am looking at the commits in > > https://github.com/apache/arrow/commits/master/java > > and trying to understand the conflict as it's being described. > > I see two non-trivial patches: ARROW-1047 and ARROW-1710. > > On each of these: > > * ARROW-1047, was there inadequate discussion on this? This PR was > open for nearly 3 weeks, all 3 stakeholders (Sidd, Bryan, Li) seemed > to approve of merging the patch. I apologize if this was not the case > * ARROW-1710: I am again seeing a lengthy discussion (this PR was up > for more than a week) and adequate opportunity for the community > review and indicate whether it should not be merged or whether more > design work is required > > If we need to revert one or both of these patches and go back to the > drawing board that is fine, but I'm trying to see where the governance > / consensus breakdown happened. It seems like perhaps there was not > enough discussion / scrutiny on the PRs themselves. Compared with > ARROW-1463, they seemed much less invasive as far as their impact on > downstream users. > > There is obviously a spectrum of developer culture from Waterfall to > Agile development style. When there is lack of progress on a > particular issue, I think that putting up a PR for discussion is a > good way to catalyze progress toward resolving a matter. > > In any case, I am supportive of whatever process helps the Java > developers arrive as expediently as possible at consensus and pushing > forward development progress quickly so that we can keep up a steady > release cycle. It's important that we make progress and grow the > community so that downstream projects can be successful consumers of > the Arrow libraries. > > Thanks > Wes > > On Wed, Nov 29, 2017 at 3:26 PM, Li Jin <ice.xell...@gmail.com> wrote: > > Sid, > > > > I agree some design and requirement discussion needs to happen. My > > generally feeling with design doc is that it's good for stating high > level > > goal but hard to track and discuss details (which I think pull request > is a > > better place for this). > > > > For instance, I have been trying to simplify class hierarchy for > Timestamp, > > Date and Time, which I first stated in the Requirement 3 in the initial > > design doc: > > https://docs.google.com/document/d/1ysZ76zritBDwkeQz3C6- > vhQwD32jEXd1kUF4T936G1U > > > > but we haven't been able to get too much discussion on that without > having > > an PR. > > > > I feel sometimes the better place to discuss this is actually in PR > itself. > > How do you feel about discuss requirement/design in the PR itself for > such > > changes? I am certainly happy to change PR description to have more > > requirement/design. > > > > Li > > > > > > > > > > > > On Wed, Nov 29, 2017 at 2:30 PM, Siddharth Teotia <siddha...@dremio.com> > > wrote: > > > >> Folks, > >> > >> Over the last couple of weeks, we have had several changes (both merged > and > >> in the pipeline) as follow-up work after ARROW-1463 was merged. > >> > >> I feel that refactoring suggestions are being proposed on-the-fly while > the > >> developer is already in progress with the code changes and it's too > late to > >> have an opinion on the changes. > >> > >> It doesn't give the reviewer enough time to understand the rationale > behind > >> the proposed changes and assess its impact downstream and most > importantly > >> have a clear idea of what all changes are being implemented so that > >> downstream consumers can understand what to expect when they rebase next > >> time. > >> > >> Two sets of such follow up changes are already merged to master. For the > >> ones in pipeline, I request people to send out a doc or spec > highlighting > >> what we are proposing to change and rationale similar to how > requirements > >> and design spec for ARROW-1463 was sent out prior to making any code > >> changes. > >> > >> Thanks, > >> Siddharth > >> >
