node_modules/url.js - Issue 29336753: Issue 3671 - Split out contentBlockerList API

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments.

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Side by Side Diff: node_modules/url.js

Issue 29336753: Issue 3671 - Split out contentBlockerList API (Closed)

Patch Set: Addressed feedback, removed some modules Created Feb. 21, 2016, 11:26 a.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 /*
	Sebastian Noack 2016/02/21 20:34:45 Please correct me if I'm wrong, as I'm not too fam Please correct me if I'm wrong, as I'm not too familiar with that whole node.js universe either, but to me it seems that node_modules is only for modules installed through npm, while own modules should go under lib. kzar 2016/02/22 12:28:06 The documentation[1] seems to say that while the l Show quoted text On 2016/02/21 20:34:45, Sebastian Noack wrote: > Please correct me if I'm wrong, as I'm not too familiar with that whole node.js > universe either, but to me it seems that node_modules is only for modules > installed through npm, while own modules should go under lib. The documentation[1] seems to say that while the lib/node/ and .node_libraries/ directories can also be used as well as node_modules/ it is for historic reasons and discouraged. I think how the files are laid out now is more or less correct, but like you I'm no expert. 1: https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders
	2 * This file is part of Adblock Plus <https://adblockplus.org/>,

	3 * Copyright (C) 2006-2016 Eyeo GmbH

	4 *

	5 * Adblock Plus is free software: you can redistribute it and/or modify

	6 * it under the terms of the GNU General Public License version 3 as

	7 * published by the Free Software Foundation.

	8 *

	9 * Adblock Plus is distributed in the hope that it will be useful,

	10 * but WITHOUT ANY WARRANTY; without even the implied warranty of

	11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

	12 * GNU General Public License for more details.

	13 *

	14 * You should have received a copy of the GNU General Public License

	15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.

	16 */

	17

	18 /** @module url */
	Sebastian Noack 2016/02/21 20:34:45 Since the existence of this module is just an impl Since the existence of this module is just an implementation detail, I think, I wouldn't want to have it documented, if we will ever generate documentation for this project. kzar 2016/02/22 12:28:06 Done. Show quoted text On 2016/02/21 20:34:45, Sebastian Noack wrote: > Since the existence of this module is just an implementation detail, I think, I > wouldn't want to have it documented, if we will ever generate documentation for > this project. Done. kzar 2016/02/22 12:28:06 Done. Show quoted text On 2016/02/21 20:34:45, Sebastian Noack wrote: > Since the existence of this module is just an implementation detail, I think, I > wouldn't want to have it documented, if we will ever generate documentation for > this project. Done.
	19

	20 "use strict";

	21

	22 exports.getBaseDomain = hostname => require("tldjs").getDomain(hostname);
	Sebastian Noack 2016/02/21 20:34:45 How about a comment that this is mimicking the url How about a comment that this is mimicking the url module in adblockpluschrome for compatibility? Sebastian Noack 2016/02/21 20:34:45 Nit: exports.getBaseDomain = require("tldjs").getD Nit: exports.getBaseDomain = require("tldjs").getDomain; kzar 2016/02/22 12:28:06 I did that originally but it didn't work. I've cha Show quoted text On 2016/02/21 20:34:45, Sebastian Noack wrote: > Nit: exports.getBaseDomain = require("tldjs").getDomain; I did that originally but it didn't work. I've changed it, what do you think? (I don't think it matters much either way.) kzar 2016/02/22 12:28:06 Done. Show quoted text On 2016/02/21 20:34:45, Sebastian Noack wrote: > How about a comment that this is mimicking the url module in adblockpluschrome > for compatibility? Done.
OLD	NEW

« lib/contentBlockerLists.js ('K') | « node_modules/filterClasses.js ('k') | no next file » | no next file with comments »