Skip to content

Migrating extendedset from Metamarkets.#3694

Merged
drcrallen merged 5 commits intoapache:masterfrom
akashdw:extendedset_migration
Jan 17, 2017
Merged

Migrating extendedset from Metamarkets.#3694
drcrallen merged 5 commits intoapache:masterfrom
akashdw:extendedset_migration

Conversation

@akashdw
Copy link
Copy Markdown
Contributor

@akashdw akashdw commented Nov 15, 2016

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

Note:

  1. External extensions might have to change.
  2. Differing license headers because extendedset is actually a fork of someone else's repository and licensed to Metamarkets under a CLA is not true.

gianm
gianm reviewed Nov 15, 2016
View reviewed changes
Comment thread NOTICE Outdated
Copy link
Copy Markdown
Contributor

@gianm gianm Nov 15, 2016

Choose a reason for hiding this comment

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

This should probably mention the original codebase too

Copy link
Copy Markdown
Contributor Author

@akashdw akashdw Nov 17, 2016

Choose a reason for hiding this comment

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

Updated the NOTICE.

@drcrallen drcrallen self-assigned this Nov 15, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Nov 15, 2016

This needs to attribute the original codebase

@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Nov 16, 2016

@fjy @gianm Found this url http://concise.svn.sourceforge.net/viewvc/concise?view=rev&revision=1 from concise paper. Unfortunately this link is broken and returning 404.

Code repo https://sourceforge.net/projects/concise/ is also empty.
Please let me know if I'm missing anything or looking in the wrong place.

Also, what do you think adding below in NOTICE (assuming original code is missing from sourceforge)

This product contains a modified version of Alessandro Colantonio's CONCISE
(COmpressed 'N' Composable Integer SEt) library, extending the functionality of
ConciseSet to use IntBuffers.

@akashdw akashdw force-pushed the extendedset_migration branch from ab02ce5 to 4f1b007 Compare November 17, 2016 19:25
@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Nov 18, 2016

@fjy @gianm Updated the notice to attribute more details of original codebase and license.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Nov 18, 2016

👍

@fjy fjy added this to the 0.9.3 milestone Nov 18, 2016
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Dec 2, 2016

NB metamx/extendedset#10 was merged in a branch upstream after this PR was filed.

@leventov
Copy link
Copy Markdown
Member

leventov commented Dec 2, 2016

@gianm no need to rebase, I'll PR into Druid after this PR (#3694) is merged.

@drcrallen
Copy link
Copy Markdown
Contributor

drcrallen commented Dec 21, 2016

@akashdw sorry for the delay, I think we're good here. If you can fix merge conflicts I can merge

@akashdw akashdw force-pushed the extendedset_migration branch from ffb097a to ff0eead Compare January 13, 2017 01:21
@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Jan 13, 2017 •
edited
Loading

@drcrallen Sorry for the delay, I was out for sometime. I rebased the pr but checkstyle is failing because of the license headers on extended set files. I can not change the license because extendedset is a fork of someone else's repository and licensed to Metamarkets under a CLA is not true.

@akashdw
Copy link
Copy Markdown
Contributor Author

akashdw commented Jan 13, 2017 •
edited
Loading

@drcrallen I added a suppress header check for extendedset files.

@drcrallen drcrallen merged commit dd0c4e2 into apache:master Jan 17, 2017
@leventov leventov mentioned this pull request Jan 25, 2017
Merged
dgolitsyn pushed a commit to metamx/druid that referenced this pull request Feb 14, 2017
@akashdw @dgolitsyn
Migrating extendedset from Metamarkets. (apache#3694)
b9c38c0 
* Migrating extendedset from Metamarkets.

* Notice change

* More details in NOTICE

* NOTICE formatting.

* suppress header checkstlye for extendedset.
@gianm gianm mentioned this pull request Jul 7, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gianm gianm gianm left review comments

Assignees

No one assigned

Labels

Improvement

Projects

None yet

Milestone

0.10.0

Development

Successfully merging this pull request may close these issues.

6 participants

@akashdw @fjy @gianm @leventov @drcrallen @cheddar