Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update d3 cloud #86

Closed
cesine opened this issue Jun 22, 2015 · 2 comments
Closed

Update d3 cloud #86

cesine opened this issue Jun 22, 2015 · 2 comments

Comments

@cesine
Copy link
Contributor

cesine commented Jun 22, 2015

Lots of other people have worked on the d3 cloud since 2013, some to add bower/amd support similar to what we did.

This issue is to go through all the pull requests and see if we can merge them, and then break off from the upstream since its not receiving updates.

https://github.com/jasondavies/d3-cloud/pulls

@cesine
Copy link
Contributor Author

cesine commented Jun 25, 2015

I'm pretty much done with looking at the pull requests. the importer https://github.com/IQAndreas/github-issues-import i used made so much activity on the upstream that jason decided to do a new version which is great, although he wrote the code himself it seems jasondavies/d3-cloud@0ed9ac8 and closed all the pull requests without merging them so its kind of hard to see which requests he took.

bottom line, we can probably keep using the upstream because its AMD compatible, bower installable and has almost all the same functionality as the pull requests.

I added some tests and travis integration to make it easier for the upstream to accept pull requests but jason didnt want to accept any extra code to maintain so I guess we will keep our fork for hosting the tests in case others want to use them.

screen shot 2015-06-24 at 8 07 44 pm

@vronvali and @louisa-bielig here are links to all the examples and the tests in case you need them when you are working.

The only difference right now is upstream is failing 3 tests out of 22, only one of which really matters.

  • it doesnt have all the words in the cloud if they don't fit (yet, maybe it will at a future date) one of the pull requests solves this but maybe not in the best way.
  • it lets invalid sizes be set (which results in an unresponsive browser which eats your battery if you actually try to render that cloud) but that jason said that input validation would bloat the codebase which means those tests aren't actually applicable to the upstream fork
    screen shot 2015-06-25 at 7 50 08 am

@cesine cesine closed this as completed Jun 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant