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

Use absolute URLs #16 #59

Merged
merged 1 commit into from
Sep 1, 2016
Merged

Conversation

jaredlockhart
Copy link
Collaborator

Fixes #16

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e0797f2 on JaredKerim-Mozilla:16 into 0aab05b on mozilla:master.

const metadata = {};
const ruleSet = rules || metadataRules;

context = context || {};
Copy link
Contributor

@pdehaan pdehaan Aug 25, 2016

Choose a reason for hiding this comment

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

nit: I think you could also just specify a default value in the function declaration:

function getMetadata(doc, ruleSet = metadataRules, context = {}) {
  ...
}

@pdehaan
Copy link
Contributor

pdehaan commented Aug 25, 2016

It looks like processors[] is an array, but not sure if you want to add any tests that have multiple processors to make sure that all processors get run and results are all applied.
Currently it looks like every ruleset only has 0-1 processors.

@pdehaan
Copy link
Contributor

pdehaan commented Aug 25, 2016

r+ w/ question about sampleHtml versus relativeHtml.

We can probably ignore the dumb processors[] and default arguments comments.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 208a7ce on JaredKerim-Mozilla:16 into 0aab05b on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 208a7ce on JaredKerim-Mozilla:16 into 0aab05b on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e7f2626 on JaredKerim-Mozilla:16 into 0aab05b on mozilla:master.

@pdehaan
Copy link
Contributor

pdehaan commented Sep 1, 2016

@jaredkerim Is this ready to merge?
I think once this lands, I can rebase my other PRs and use processors to convert keywords into array, etc.

@jaredlockhart jaredlockhart merged commit cef465e into mozilla:master Sep 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants