Skip to content
This repository has been archived by the owner on Oct 22, 2019. It is now read-only.

Use metadata to inject the experiment.html when "experiment": true #73

Merged
merged 1 commit into from
Mar 11, 2016

Conversation

juliantoledo
Copy link
Collaborator

No description provided.

@juliantoledo juliantoledo force-pushed the inject-experimental2samples branch 3 times, most recently from a810eae to 3f49b20 Compare March 10, 2016 14:16
@juliantoledo
Copy link
Collaborator Author

@sebastianbenz fixed the two questions I asked. PTAL

@@ -60,6 +60,15 @@ module.exports = function(templateRoot, template) {
const example = ExampleFile.fromPath(file.path);
const nextExample = example.nextFile();

// Experimet must be injected in the first section only
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: typo in Experiment

@@ -13,9 +13,12 @@
<body>
<div class="body">
{{> header.html}}
{{#sections}}
{{#data.sections}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need the additional 'data' indirection?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In order to pass:
"experimental": function() {
return (idx++ == 0 && document.metadata.experiment);
Experimental needs to be inside the first section.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@juliantoledo understood, but couldn't we just add the function as a field in the args object?

@juliantoledo juliantoledo force-pushed the inject-experimental2samples branch 3 times, most recently from 1a265d6 to b2a7f57 Compare March 10, 2016 15:03
@jkingyens
Copy link
Contributor

I have a commit based on this branch that passes this experimental flag to validation task and skips validation for experimental examples. I can wait for this PR to land in master or add the commit here. I just wanted to be sure this PR satisfied requirements for that.

@juliantoledo
Copy link
Collaborator Author

@jkingyens Sounds good, the metadata is already supported in the document parser. I will finish this PR in a bit.

@juliantoledo
Copy link
Collaborator Author

@sebastianbenz fixed your comments and added the metadata instructions to the readme.

};

if (document.metadata.experiment && (!document.metadata.component ||
document.metadata.component.length === 0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if (document.metadata.experiment && !document.metadata.component) is enough:

> false == ''
true

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

};

if (document.metadata.experiment && (!document.metadata.component ||
document.metadata.component.length === 0)) {
throw new Error("Example (" + example.url() +
Copy link
Collaborator

Choose a reason for hiding this comment

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

throw a gulp exception: throw new PluginError(plugin: 'compile-example', message: '...');

@sebastianbenz
Copy link
Collaborator

LGTM - good work!

sebastianbenz added a commit that referenced this pull request Mar 11, 2016
Use metadata to inject the experiment.html when "experiment": true
@sebastianbenz sebastianbenz merged commit 1cda594 into master Mar 11, 2016
@juliantoledo juliantoledo deleted the inject-experimental2samples branch March 15, 2016 13:18
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