Skip to content
This repository has been archived by the owner on Jul 27, 2020. It is now read-only.

Added Docs and Examples! #16

Closed
wants to merge 5 commits into from
Closed

Conversation

JimLynchCodes
Copy link

Ngrx is an amazing Reactive-Redux implementation for Angular 2. Ngrx/db is an important tool our toolbag for preserving the state when the page is closed or refreshed. This repo has gotten NO love at all in terms of the README, and a lot of people have been / still are very confused about it. Imo we should all just start using it and work out the bugs (if there are any) when people open issues, just like any other project.

Anyway, I'm open to feedback and criticism if anything I wrote is totally wrong. Just let me know or change it.

Thanks,
Jimbo

README.md Outdated

### The RxJS Powered Local Storage Solution for Angular2 apps

Most browsers have a local data storage mechanism called IndexedDB that allows your application to persist data after the browser is closed or refreshed. Ngrx/db is a library that provides a convenient API for storing and retrieving this data, and it fits nicely into Ngrx's reactive redux paradigm.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of "Ngrx/db" lets go with "@ngrx/db" throughout

@JimLynchCodes
Copy link
Author

ok. 👍

I just changed to my branch to have the all lowercase capitalization ngrx/db.

@michaeljota
Copy link

I'm just watching this, but .idea folder is necessary? Can/Should it be removed?

@JimLynchCodes
Copy link
Author

Ah sorry. I didn't realize I added that. It's an artifact created by WebStorm / intellij. You can delete it. :+1

@e-cloud
Copy link

e-cloud commented Feb 21, 2017

add a rule in .gitignore will avoid it.

@JimLynchCodes
Copy link
Author

JimLynchCodes commented Feb 21, 2017

ok. I just deleted the .idea folder so it's no longer included in this PR. :)

@JimLynchCodes
Copy link
Author

Hey, how is it looking now? Can we merge this? :)

@JimLynchCodes
Copy link
Author

Can someone merge this?

@michaeljota
Copy link

I'm not a collaborator or anything, but this seems more like an internal project, that something that someone else could really use.

It looks really good, but I don't think this is the right time to change the readme yet.

@michaeljota
Copy link

You may want something, yet be unable to use it. As this.

@michaeljota
Copy link

The fact that someone is using it, it's not proof to say it should be used.

@JimLynchCodes
Copy link
Author

JimLynchCodes commented Mar 3, 2017

Well I'm using it along with a few others at least, and it's working fine for me. It would be nice to know why it says don't use me yet it linked to from other ngrx pages as if it is a legitimate thing...

What would be sufficient proof that it should be used? I think open source should be about being open, and putting up a "don't use me" sign is the opposite of that.

BTW anyone who actually wrote this library (@mikeryan52 @brandonroberts @robwormald) feel free to chime in...

@michaeljota
Copy link

Nope. I mean, the people who actually wrote this say "don't use me yet".

@michaeljota
Copy link

🙄🙄🙄🙄 You must be the rudest person I ever chat here in Github. They have no obligation to even have their own repo documented in anyhow. Thanks you for your README, if that's what you want, but I hope this gets closed, along with your duplicate issue #16 :).

@JimLynchCodes
Copy link
Author

@michaeljota Please stop trolling this thread.

README.md Outdated

### RxJS powered IndexedDB for Angular2 apps
## __**USE ME NOW!**__
Copy link

Choose a reason for hiding this comment

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

I don't think this is necessary, the fact that "Don't use me yet." was removed is sufficient to indicate it is now usable

@somombo
Copy link

somombo commented Mar 8, 2017

Agreed, @michaeljota your personal attacks don't add any value to this or any project. I think @JimTheMan should be commended for atleast trying to contribute something. And the open source maintainers do have some level of obligation, IMO to atleast respond "we have seen your comments/PR, but we are busy at the moment and will respond at such and such a time"

Good work @JimTheMan

@michaeljota
Copy link

@somombo Just for clarification, I'm not personal attacking him. I'm watching this whole repo, so I can track progress. If you look in issues, you could see an open issue that only refers to merge this, still open, PR.

So I'm just leading to the fact that the people behind this project may not want to make this public usable yet, and this guy keep pushing to merge something as it is some kind of obligation for this organization to merge every PR.

I just want a notification here to be related with progress. I'm sorry if there is any misunderstanding.

@varghesep
Copy link

varghesep commented Mar 25, 2017

@JimTheMan's opinions are valid. A coordinated effort can make this library usable.

@robwormald
Copy link
Contributor

@JimTheMan thanks for the PR. i'm merging it now and then i'll be migrating this repo to the monorepo @ https://github.com/ngrx/platform

to everyone commenting in here:

ngrx/db began and remains an experimental thing. IndexedDB is a non-trivial API, and notoriously difficult to use consistently across different browsers. I'm not comfortable committing to the full support we give the other ngrx projects until it's been much more extensively tested and proven, which is why it has not previously had docs or a readme.

I, frankly, don't have the hours in the day to make that happen right now. If the community would like to make that happen, I am all for it and would welcome your help.

@MattiJarvinen
Copy link

@robwormald hmm.... about migrating to monorepo? What would be needed? Do we get a roadmap what PRs have to happen? List of issues with IndexedDB to be tested etc?

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.

8 participants