Skip to content

Migrating bytebuffercollections from Metamarkets.#3647

Merged
drcrallen merged 5 commits intoapache:masterfrom
akashdw:migration
Nov 11, 2016
Merged

Migrating bytebuffercollections from Metamarkets.#3647
drcrallen merged 5 commits intoapache:masterfrom
akashdw:migration

Conversation

@akashdw
Copy link
Copy Markdown
Contributor

@akashdw akashdw commented Nov 2, 2016

Migrating bytebuffercollections from metamarkets. This migration will be helpful to deal problems around having separate PRs and release cycle of Druid, extendedset and bytebuffercollections. This PR is continuation of #3585.

Note: External extensions might have to change.

@cheddar
Copy link
Copy Markdown
Contributor

cheddar commented Nov 2, 2016

Please note the differing license headers on extendedset and bytebuffercollections files. extendedset is actually a fork of someone else's repository, given that it is a fork already, the statement that it was "licensed to Metamarkets under a CLA" is not true, so we instead opted to keep the same license header.

@drcrallen
Copy link
Copy Markdown
Contributor

@cheddar given that, can we split the "purely MMX CLA" into its own PR for easier merging?

I'm a bit concerned about how we can make sure we can maintain a proper licensing header for things that did not originate under a MMX CLA

@akashdw akashdw changed the title Migrating extendedset and bytebuffercollections from Metamarkets. Migrating bytebuffercollections from Metamarkets. Nov 2, 2016
@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Nov 2, 2016

@drcrallen @cheddar Removed extendedset module.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Nov 8, 2016

@akashdw there are some code conflicts

@fjy fjy added this to the 0.9.3 milestone Nov 8, 2016
@fjy fjy added the Improvement label Nov 8, 2016
@cheddar
Copy link
Copy Markdown
Contributor

cheddar commented Nov 8, 2016

@fjy the code conflicts will happen on almost every PR merge because imports. It would be great if we could get signoff on the general idea from @drcrallen or @leventov as representatives of MMX (given that the code is coming from an MMX repo) before resolving the conflicts. Otherwise, there will be constant chasing of conflicts.

@leventov
Copy link
Copy Markdown
Member

leventov commented Nov 8, 2016

@cheddar I leave this to @drcrallen.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dangling <p>s apparently get in the way of the building of javadoc for Druid. They should be cleaned up. It should be acceptable to just replace <p> with a space.

@drcrallen
Copy link
Copy Markdown
Contributor

Other than what @cheddar mentioned, this looks good to me. I can do the merge once the conflicts are cleaned up.

I did some more digging on the <p> thing and I think I had it backwards. This is in the master pom: https://github.com/druid-io/druid/blob/druid-0.9.2-rc2/pom.xml#L787 which I think prevents the problem I was describing.

I think my brain was backwards, where bytebuffer-collections would fail compiling if java8 was used.

Looking through druid source code there are a few instances of <p> which should have otherwise given us compile problems.

So you can fix the merge conflicts and I'll merge this promptly assuming another committer can give the other plus one.

@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Nov 8, 2016

@cheddar @drcrallen @fjy I uploaded a new patch set to resolve code conflicts and removed <p> from bytebuffer-collections.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Nov 8, 2016

👍

@drcrallen drcrallen merged commit 3e40849 into apache:master Nov 11, 2016
@leventov
Copy link
Copy Markdown
Member

leventov commented Dec 7, 2016

@akashdw could you also please remove bytebuffer-collections dependency from the root pom?

fundead pushed a commit to fundead/druid that referenced this pull request Dec 7, 2016
* Migrating  bytebuffercollections from Metamarkets.

* resolving code conflicts and removing <p> from bytebuffer-collections.
@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Dec 7, 2016

@leventov Sure, I will create a pr for the pom cleanup.

@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Dec 8, 2016

@leventov pr for root pom cleanup, #3764

dgolitsyn pushed a commit to metamx/druid that referenced this pull request Feb 14, 2017
* Migrating  bytebuffercollections from Metamarkets.

* resolving code conflicts and removing <p> from bytebuffer-collections.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants