{"action":"created","issue":{"url":"https://api.github.com/repos/poetapp/node/issues/170","repository_url":"https://api.github.com/repos/poetapp/node","labels_url":"https://api.github.com/repos/poetapp/node/issues/170/labels{/name}","comments_url":"https://api.github.com/repos/poetapp/node/issues/170/comments","events_url":"https://api.github.com/repos/poetapp/node/issues/170/events","html_url":"https://github.com/poetapp/node/pull/170","id":334649165,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk2NTY5MTgz","number":170,"title":"Implement Claim Batching - directoryCollection module proposal","user":{"login":"kennylavender","id":5175120,"node_id":"MDQ6VXNlcjUxNzUxMjA=","avatar_url":"https://avatars1.githubusercontent.com/u/5175120?v=4","gravatar_id":"","url":"https://api.github.com/users/kennylavender","html_url":"https://github.com/kennylavender","followers_url":"https://api.github.com/users/kennylavender/followers","following_url":"https://api.github.com/users/kennylavender/following{/other_user}","gists_url":"https://api.github.com/users/kennylavender/gists{/gist_id}","starred_url":"https://api.github.com/users/kennylavender/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kennylavender/subscriptions","organizations_url":"https://api.github.com/users/kennylavender/orgs","repos_url":"https://api.github.com/users/kennylavender/repos","events_url":"https://api.github.com/users/kennylavender/events{/privacy}","received_events_url":"https://api.github.com/users/kennylavender/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-06-21T20:40:06Z","updated_at":"2018-06-21T23:53:20Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/poetapp/node/pulls/170","html_url":"https://github.com/poetapp/node/pull/170","diff_url":"https://github.com/poetapp/node/pull/170.diff","patch_url":"https://github.com/poetapp/node/pull/170.patch"},"body":"part of #142 \r\n\r\nProposing the following file change to the directory collection module and the related changes to make it possible.\r\n\r\nHere were my goals\r\n\r\n### Reduce dependencies and coupling\r\n#### No database\r\nRemove the need for the directory collection module to know about the database because it only needs to know about the single collection.\r\n\r\n#### Receive a collection, don't define one.\r\nThe directory module does not need to know about the specific collection name if it just receives a collection as an argument. This allows the higher level modules ( Router.js or Storage.js ) to define what the collection names are and which modules manipulate those collections. It also means we could reuse a collection module because we can create multiple instances and give them different collections. We might be able to create a collection module that handles multiple use cases since lots of our hash/retry/attempt logic is looking very similar.\r\n\r\n### Responsibility\r\nThe directory collection module should handle all logic related to the collection, I think setting the unique indexes should be done from this module, not in other modules. This makes logic related to the unique indexes testable with integration tests of this single module. \r\n\r\nHandling the responsibility in the directory collection module\r\nhttps://github.com/poetapp/node/blob/claim-batching-proposal-01/src/Storage/DirectoryCollection.ts#L37\r\n\r\nVs handling it externally\r\nhttps://github.com/poetapp/node/blob/claim-batching-proposal-01/src/Storage/Storage.ts#L80-L84\r\n\r\n### Typescript Noise\r\nremove typescript typing noise, try to separate types from functions, still working on this, not sure how I like the current code. Being new to typescript please point out anything you see that could be improved here for sure.\r\n\r\n### Easier Testing, less mocking\r\nCompleting the above items should make the directory collection module much easier to test with integration tests if we decide to do so.\r\n\r\n\r\n@lautarodragan please take a look, let me know if you like, don't like these changes and suggestions, thanks!\r\n\r\nIf you like these changes we can apply them to the other modules in the future."},"comment":{"url":"https://api.github.com/repos/poetapp/node/issues/comments/399278369","html_url":"https://github.com/poetapp/node/pull/170#issuecomment-399278369","issue_url":"https://api.github.com/repos/poetapp/node/issues/170","id":399278369,"node_id":"MDEyOklzc3VlQ29tbWVudDM5OTI3ODM2OQ==","user":{"login":"lautarodragan","id":2279687,"node_id":"MDQ6VXNlcjIyNzk2ODc=","avatar_url":"https://avatars2.githubusercontent.com/u/2279687?v=4","gravatar_id":"","url":"https://api.github.com/users/lautarodragan","html_url":"https://github.com/lautarodragan","followers_url":"https://api.github.com/users/lautarodragan/followers","following_url":"https://api.github.com/users/lautarodragan/following{/other_user}","gists_url":"https://api.github.com/users/lautarodragan/gists{/gist_id}","starred_url":"https://api.github.com/users/lautarodragan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lautarodragan/subscriptions","organizations_url":"https://api.github.com/users/lautarodragan/orgs","repos_url":"https://api.github.com/users/lautarodragan/repos","events_url":"https://api.github.com/users/lautarodragan/events{/privacy}","received_events_url":"https://api.github.com/users/lautarodragan/received_events","type":"User","site_admin":false},"created_at":"2018-06-21T23:53:20Z","updated_at":"2018-06-21T23:53:20Z","author_association":"MEMBER","body":"> Thanks, I caught on to this for the most part ;) I have been flip/flopping on this personally I like the idea of calling the individual files modules since that is how node refers to them, and then calling the groupings of modules 'features' or something else, but modules also makes sense there as well ¯_(ツ)_/¯ idk. This sort of brings up the idea that maybe we could define something as a team and document it?\r\n\r\nYeah I'm thinking the same thing, we should document it. I've been thinking about moving them to a new folder, `src/modules/...`, so it's clear they are modules and they are not the same as `Helpers`, `Messaging` and `Extensions`.\r\n\r\nAnd you're right - hadn't thought of it before, but it's true that to NodeJS those are modules, so it's even more confusing. Can't come up with a proper name though. In reality they are separate applications, but calling them so would be confusing since we're treating them as one, too."}}
{"actor":{"display_login":"lautarodragan"}}