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

docs(decision-tree-widget): implement Operator Decision Tree widget #1406

Closed
wants to merge 2 commits into from

Conversation

staltz
Copy link
Member

@staltz staltz commented Mar 1, 2016

Implements the Operator Decision Tree widget, and add to both doc/index.md and doc/operators.md
Notice specially doc/decision-tree-widget/tree.yml which is the decision tree data encoded in YAML. Probably the decision tree YAML file still needs review and improvement, but we could update it separately from this PR.

To see this page live, open http://rxjs5-esdoc-decision-tree.surge.sh/

choose

@benlesh
Copy link
Member

benlesh commented Mar 1, 2016

That is super cool.

@kwonoj
Copy link
Member

kwonoj commented Mar 1, 2016

Love this too. 💘

@todoubaba
Copy link

It seems the operator's url is not correct, such as mapTo links to http://rxjs5-esdoc-decision-tree.surge.sh/class/es6/Observable.js~Observable.html.

@staltz
Copy link
Member Author

staltz commented Mar 1, 2016

@todoubaba The links are correct, but they refer to a future version of the Observable.html page, we're depending on PR #1376 merged first.

@todoubaba
Copy link

Yeah, looking forward that 👍

@kwonoj
Copy link
Member

kwonoj commented Mar 3, 2016

❤️ Change looks good to me.

@staltz
Copy link
Member Author

staltz commented Mar 3, 2016

I need to rebase, apparently

@staltz
Copy link
Member Author

staltz commented Mar 3, 2016

Rebased, but it will conflict with #1385 when that is merged. (and will need another rebase)

@staltz
Copy link
Member Author

staltz commented Mar 4, 2016

Rebossed.

"dependencies": {
"@motorcycle/core": "^1.1.0",
"@motorcycle/dom": "^1.2.1",
"most": "^0.18.1",
Copy link
Member

Choose a reason for hiding this comment

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

LOL... what?! 👎

Copy link
Member

Choose a reason for hiding this comment

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

You're just asking for @briancavalier to come tease me aren't you?

Choose a reason for hiding this comment

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

lgtm 👍 🚢

Copy link
Contributor

Choose a reason for hiding this comment

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

This is so good, it's nearly up there with when that gentleman asked @Blesh on his article, if he'd used RxJS!!
#acidental-trolling :trollface:

@benlesh
Copy link
Member

benlesh commented Mar 4, 2016

So, you know what I'm going to say. This is really cool. We should probably implement it with RxJS 5 and annotate the code. :P

LOLOLOL... Too funny, @staltz

@staltz
Copy link
Member Author

staltz commented Mar 4, 2016

So, in reality it's not trolling and it's absolutely serious stuff. Cycle.js doesn't currently work with RxJS v5, we're working on getting that support up. In the meanwhile, RxJS v4 is quite huge in size to include to this page, specially because we already have full RxJS v5 loaded in the page. So I went for something small in size, and I chose a version of Cycle.js called Motorcycle.js, focused on performance.

If you'd prefer for PR purposes that I just use RxJS v4 even if it's larger, fine by me. Or, I can place the whole decision tree widget elsewhere, make it generic for any purposes, then just import it here. So, your call.

@benlesh
Copy link
Member

benlesh commented Mar 4, 2016

If you'd prefer for PR purposes that I just use RxJS v4 even if it's larger, fine by me.

Honestly, that's probably better. But ideally, we'd have it in RxJS 5 and it would be annotated.

Could write it in Angular 2 ;) haha.

@staltz
Copy link
Member Author

staltz commented Mar 4, 2016

Could write it in anything. I just wanted to get it done.

But ideally, we'd have it in RxJS 5 and it would be annotated.

Will do when the support is out. I'll update this PR with Rx4 to make y'all happy.

@xgrommx
Copy link

xgrommx commented Mar 4, 2016

@staltz Now you can use only what you want of Rx instead of whole library https://github.com/Reactive-Extensions/RxJS/tree/master/src/modular

@staltz
Copy link
Member Author

staltz commented Mar 5, 2016

I updated the PR to use Rx v4, and uglifyjs.

@staltz
Copy link
Member Author

staltz commented Mar 8, 2016

Rebased

@kwonoj
Copy link
Member

kwonoj commented Mar 10, 2016

Looks good to me (but another rebase... 😓 )

@staltz
Copy link
Member Author

staltz commented Mar 10, 2016

Rebombed. :)

@staltz
Copy link
Member Author

staltz commented Mar 15, 2016

@Blesh are you serious about this PR having to use RxJS v5 for dogfooding purposes? Just want to know what are your intentions because I could honestly convert this to a pseudo-Cycle.js app, since it's so small I can give up a bit on the actual framework and just apply the same architectural ideas.

@staltz
Copy link
Member Author

staltz commented Mar 17, 2016

Ok, I just did what I said in my previous comment, so now it should be fully Blesh-Compliant Enterprise Software.

@staltz
Copy link
Member Author

staltz commented Mar 29, 2016

Rebased ✅

@kwonoj
Copy link
Member

kwonoj commented Mar 29, 2016

I think change's ok to be checked in, will check this in around tomorrow unless other suggestion's around.

@kwonoj
Copy link
Member

kwonoj commented Mar 29, 2016

Merged with ffe71df, 🎉 @staltz

@kwonoj kwonoj closed this Mar 29, 2016
@staltz staltz deleted the esdoc-decision-tree branch March 30, 2016 07:14
@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants