Closed southbite closed 2 years ago
Hi Jens, It is marked (WIP) - so still busy ;)
On Fri, 05 Nov 2021 at 08:29, JensEggers @.***> wrote:
Not sure if I'm supposed to be reviewing this PR yet, but just an initial comment: we need to be able to exclude NEDB from our build. The data provider wrappers themselves are probably small and could always be present, but I think that both NEDB and LokiJS should be optional dependencies. Not completely sure how to manage optional dependencies, except that they should only be required if configured. Perhaps this is a good-enough failure point as we'll soon know if the database is missing. So for this repo both happn-nedb and lokijs could be devDeps.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/happner/happn-3/pull/357#issuecomment-961656683, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAO6EBQ5CHO6FO7ZNN6775LUKN2VDANCNFSM5HLVAQ2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
--
SIMON BISHOP
DIRECTOR
+27 (0) 83 282 0703
| @. @.>
twitter: @tenacious http://www.twitter.com/_tenacious_
| skype: sjbishop
Trustlab, 2nd floor, Castle Mews, 16a Newmarket street, Cape Town
www.tenacious.digital
Not sure if I'm supposed to be reviewing this PR yet, but just an initial comment: we need to be able to exclude NEDB from our build. The data provider wrappers themselves are probably small and could always be present, but I think that both NEDB and LokiJS should be optional dependencies. Not completely sure how to manage optional dependencies, except that they should only be required if configured. Perhaps this is a good-enough failure point as we'll soon know if the database is missing. So for this repo both happn-nedb and lokijs could be devDeps.