Complete filter #2

Merged
senaduka merged 11 commits from complete-filter into master 2017-12-14 14:22:32 +01:00
senaduka commented 2017-12-11 18:24:52 +01:00 (Migrated from gitlab.com)

Created by: GotPPay

*Created by: GotPPay*
senaduka commented 2017-12-11 22:13:44 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

No need for two if statements, you can use &&

*Created by: MirnaM* No need for two if statements, you can use &&
senaduka commented 2017-12-12 09:55:35 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

there is a lot of nesting here. can you extract some of the blocks to separate methods, maybe this one

*Created by: MirnaM* there is a lot of nesting here. can you extract some of the blocks to separate methods, maybe this one
senaduka commented 2017-12-12 09:58:19 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

Maybe extract this callback to a method processDomains, and call it in getRawDomainList?

*Created by: MirnaM* Maybe extract this callback to a method processDomains, and call it in getRawDomainList?
senaduka commented 2017-12-12 10:05:37 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

This is hard to read. Maybe you can split this callback into few methods that do it's own thing, for example comparing domains with yesterday, and removing domains from expired list can be separate methods. Also, no need to pass this as a parameter

*Created by: MirnaM* This is hard to read. Maybe you can split this callback into few methods that do it's own thing, for example comparing domains with yesterday, and removing domains from expired list can be separate methods. Also, no need to pass this as a parameter
senaduka commented 2017-12-13 16:25:00 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

Maybe move this map to cleanExpired, and add expired domains to array, and then call insert on resulting array. This will cause insert to be called only once, not one time for every domain, since db.collection.insert accepts array of documents. This way only one promise will be returned.

*Created by: MirnaM* Maybe move this map to cleanExpired, and add expired domains to array, and then call insert on resulting array. This will cause insert to be called only once, not one time for every domain, since db.collection.insert accepts array of documents. This way only one promise will be returned.
senaduka commented 2017-12-13 16:26:41 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

This will be resolved before db insert is finished. do db.collection('expired_list').insert(domains, () => {resolve();}) if there are expired domains

*Created by: MirnaM* This will be resolved before db insert is finished. do db.collection('expired_list').insert(domains, () => {resolve();}) if there are expired domains
senaduka commented 2017-12-13 16:27:16 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

If there are expired domains, this will be resolved before http.get is finished.

*Created by: MirnaM* If there are expired domains, this will be resolved before http.get is finished.
senaduka commented 2017-12-13 16:28:21 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

No need to wrap this in promise

*Created by: MirnaM* No need to wrap this in promise
senaduka commented 2017-12-13 16:28:31 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

No need to wrap this in promise

*Created by: MirnaM* No need to wrap this in promise
senaduka commented 2017-12-13 16:36:29 +01:00 (Migrated from gitlab.com)

Created by: MirnaM

maybe push to an array, and after all domains are checked call remove in the end. you can write query with $in

*Created by: MirnaM* maybe push to an array, and after all domains are checked call remove in the end. you can write query with $in
senaduka commented 2017-12-14 02:52:49 +01:00 (Migrated from gitlab.com)

Created by: GotPPay

This removal was wrong, since it removed all domains, not checking TLD (.se or .nu or .com), and with new filter, I don't know is it practical to separate different domains in different arrays to remove using $in query

*Created by: GotPPay* This removal was wrong, since it removed all domains, not checking TLD (.se or .nu or .com), and with new filter, I don't know is it practical to separate different domains in different arrays to remove using $in query
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: senaduka/old-domene-svedska#2