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

fix(Rx): remove kitchenSink and DOM, let Rx exports all #1663

Merged
merged 1 commit into from
Apr 30, 2016

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Apr 28, 2016

Description:
This PR removes Rx.KitchenSink, let Rx exports everything instead as well as does not populate build anymore.

Related issue (if exists):

closes #1650

"./doc/asset/devtools-welcome.js",
"./doc/scripts/custom-manual-styles.js",
"./doc/decision-tree-widget/dist/decision-tree-widget.min.js",
"./doc/scripts/setup-rx-script.js"
"./doc/decision-tree-widget/dist/decision-tree-widget.min.js"
Copy link
Member Author

Choose a reason for hiding this comment

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

/cc @staltz for document generate script related changes.

Copy link
Member

Choose a reason for hiding this comment

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

@kwonoj is this blocking? Because the rest of this PR looks great to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, for visibility just in case if I miss anything. esdoc script is working on my local machine with this changes.

@benlesh
Copy link
Member

benlesh commented Apr 29, 2016

@kwonoj can you get rid of Rx.DOM.ts as well? We can just merge it into Rx.ts too. It's silly and confusing to have two files where one is a superset of the other, IMO.

@kwonoj
Copy link
Member Author

kwonoj commented Apr 29, 2016

Will do that as well. just got back and caught conversation's ongoing.

@kwonoj
Copy link
Member Author

kwonoj commented Apr 29, 2016

quick confirmation @Blesh - Rx suppose to not to export Rx.dom specific such as requestanimationframeScheduler, etcs, is that correct? or just export everything?

@kwonoj kwonoj changed the title chore(Rx): remove kitchenSink, let Rx exports all chore(Rx): remove kitchenSink and DOM, let Rx exports all Apr 30, 2016
@kwonoj
Copy link
Member Author

kwonoj commented Apr 30, 2016

Updated PR to remove DOM as well. @Blesh - for now I made let Rx exports everything, let me know if this isn't expected to drop off some of exports.

@benlesh
Copy link
Member

benlesh commented Apr 30, 2016

LGTM :shipit:

@kwonoj
Copy link
Member Author

kwonoj commented Apr 30, 2016

Thanks for review / confirmation. I'll check this in late tonight to tomorrow.

@kwonoj kwonoj force-pushed the refactor-rx branch 2 times, most recently from e9ec2f3 to 033497d Compare April 30, 2016 02:26
@benlesh
Copy link
Member

benlesh commented Apr 30, 2016

@kwonoj, be sure there is a breaking change message in the commit message. Since we're not putting those files out anymore. Will probably have to prefix it fix out feat and not chore

@kwonoj
Copy link
Member Author

kwonoj commented Apr 30, 2016

ah, thanks for pointing out. Let me update commit message now.

@kwonoj kwonoj changed the title chore(Rx): remove kitchenSink and DOM, let Rx exports all fix(Rx): remove kitchenSink and DOM, let Rx exports all Apr 30, 2016
@kwonoj
Copy link
Member Author

kwonoj commented Apr 30, 2016

updated commit message as well.

@benlesh
Copy link
Member

benlesh commented Apr 30, 2016

"Let Rx export all"

:)

closes ReactiveX#1650

BREAKING CHANGE: `Rx.kitchenSink` and `Rx.DOM` are removed, `Rx`
export everything.
@kwonoj kwonoj merged commit f5090b4 into ReactiveX:master Apr 30, 2016
@kwonoj kwonoj deleted the refactor-rx branch April 30, 2016 07:05
@kwonoj
Copy link
Member Author

kwonoj commented Apr 30, 2016

Merged with f5090b4.

@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.

Proposal: No more KitchenSink
2 participants