Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ES2015 classes instead of having prototype-based functions #73

Merged
merged 28 commits into from
Oct 22, 2016

Conversation

tomasko126
Copy link
Member

This change will practically improve readability of the code.
An algorithm for showing tabs in Options has also been reworked, which as a result improves performance.

@kpeckett Could you please review the changes I've made? Thanks 😀

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
…rectly by a resourceblock page

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Solution was to not execute scripts which have already been executed
An algorithm, which shows tabs has also been refactored
Empty constructors in filters.js file have been deleted
catblock.js and catblock.css files have been moved

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
…fari 6.x

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
…pport for already unsupported versions of Safari by Apple

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
@tomasko126 tomasko126 added the enhancement This is an enhancement to the extension label Sep 6, 2016
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
@tomasko126
Copy link
Member Author

tomasko126 commented Sep 7, 2016

@kpeckett I have just figured out, that I might have missed to add some arguments to some methods as in an example below.

As you can see, there is missing an enabled argument in brackets following after setEnabled.
snimka obrazovky 2016-09-07 o 9 54 39

I will compare the code from master branch and this branch and will let you know in advance, when you can start reviewing this PR.

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
@tomasko126 tomasko126 added this to the 1.5 milestone Sep 8, 2016
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
throw new Error("Implemented by subclass. Call callback with up-to-date listings.");
}
};

}

// Channel containing hard coded cats loaded from disk.
// Subclass of Channel.
Copy link
Member

Choose a reason for hiding this comment

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

Second comment not needed as it is now explicit in the code.

@@ -238,23 +239,26 @@ AprilFoolsCatsChannel.prototype = {
L(109, 385, "tall6.jpg")
]);
}
};

}


// Abstract base class for Flickr-based Channels.
// Subclass of Channel.
Copy link
Member

Choose a reason for hiding this comment

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

Also here as well

@@ -301,55 +305,51 @@ FlickrChannel.prototype = {
}
return result;
}
};
}

// Channel pulling from Flickr search results.
// Subclass of FlickrChannel.
Copy link
Member

Choose a reason for hiding this comment

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

And again

var that = this;
this._api("flickr.photos.search", { text: this._query }, function(resp) {
callback(that._toListings(resp.photos));
});
}
};
}

// Channel pulling from a Flickr photoset.
// Subclass of FlickrChannel.
Copy link
Member

Choose a reason for hiding this comment

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

I'll just let you find the rest. Do a git grep subclass or something.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kpeckett I've removed all those comments.

if (this._subscriptions[id].subscribed) {
//if forceFetch, set the last update timestamp of the malware to zero, so it's updated now.
if (forceFetch) {
this._subscriptions.malware.last_update = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Was this completely removed? I can't tell where it's moved to.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kpeckett Nope, it wasn't.

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
@tomasko126 tomasko126 merged commit 06062a2 into master Oct 22, 2016
@tomasko126 tomasko126 deleted the es2015-classes branch October 22, 2016 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement to the extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants