Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Remove ws, update #309

Merged
merged 3 commits into from
Mar 12, 2015
Merged

Remove ws, update #309

merged 3 commits into from
Mar 12, 2015

Conversation

ralphtheninja
Copy link
Member

I cleaned up some stuff and added some links in README to point to the discussion behind the removal and also a link to the wiki page.

@ralphtheninja ralphtheninja mentioned this pull request Mar 11, 2015
3 tasks
@ralphtheninja
Copy link
Member Author

Let me know if there is there anything else that should be added or needs tidying up before this can be merged.

@ralphtheninja
Copy link
Member Author

Ping @jcrugzz

@jcrugzz
Copy link
Contributor

jcrugzz commented Mar 12, 2015

Sorry! Got caught up this past couple days. Ill add my thoughts :). Thanks for spearheading this through!


TODO: talk about performance and multiple writeStream implementations
Also, check out the [modules page](https://github.com/rvagg/node-levelup/wiki/Modules) in the wiki for examples on write stream modules in userland.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a specific write-streams section to link to directly to make it a bit easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeap sounds good. I thought about it yesterday, but since we don't have any section like that I threw in a more generic link for now.

add links to issue page discussion the removal and also
link to the wiki where more information on alternative
userland write stream approaches can be found
@ralphtheninja
Copy link
Member Author

@jcrugzz @mcollina Updated README based on your feedback and also added write-stream section to the wiki https://github.com/rvagg/node-levelup/wiki/Modules#write-streams

@jcrugzz
Copy link
Contributor

jcrugzz commented Mar 12, 2015

@ralphtheninja LGTM 👍

@ralphtheninja
Copy link
Member Author

@jcrugzz Aaah you added some stuff to the wiki. Sweet! :)

@jcrugzz
Copy link
Contributor

jcrugzz commented Mar 12, 2015

@ralphtheninja yes indeed! :)

@mcollina
Copy link
Member

LGTM

Il giorno gio 12 mar 2015 alle ore 16:56 Jarrett Cruger <
[email protected]> ha scritto:

@ralphtheninja https://github.com/ralphtheninja yes indeed! :)


Reply to this email directly or view it on GitHub
#309 (comment).

ralphtheninja added a commit that referenced this pull request Mar 12, 2015
@ralphtheninja ralphtheninja merged commit b826ca4 into Level:remove-ws Mar 12, 2015
@rvagg rvagg removed the in progress label Mar 12, 2015
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.

5 participants