Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

feat(mc): #2017 Add blocking to snippets.js #2992

Merged
merged 1 commit into from
Jul 27, 2017

Conversation

k88hudson
Copy link
Contributor

This adds blocking functionality for snippets that can be used by the payload JS

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.407% when pulling e3b93b2 on k88hudson:gh2017 into 580602e on mozilla:master.

Copy link
Contributor

@sarracini sarracini left a comment

Choose a reason for hiding this comment

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

Nice, just 2 nits

@@ -22,7 +22,7 @@ class SnippetsMap extends Map {
return this._dbTransaction(db => db.put(value, key));
}

delete(key, value) {
delete(key, valuatoe) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this is a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, there isn't a value for delete anyway...

}
let blockList = this.blockList;
if (!blockList.includes(id)) {
blockList = [id].concat(blockList);
Copy link
Contributor

@sarracini sarracini Jul 27, 2017

Choose a reason for hiding this comment

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

are you not able to just push id onto blockList?

Copy link
Contributor Author

@k88hudson k88hudson Jul 27, 2017

Choose a reason for hiding this comment

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

Yeah I could, I was in no-mutate redux mode I guess

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.407% when pulling 5f5621a on k88hudson:gh2017 into 580602e on mozilla:master.

@sarracini sarracini merged commit 72c931c into mozilla:master Jul 27, 2017
@as-pine-proxy
Copy link
Collaborator

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants