Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

fix: validation schema (schema-utils) - #527

Merged
michael-ciniawsky merged 1 commit into
masterfrom
issue-524
Jun 7, 2017
Merged

michael-ciniawsky merged 1 commit into
masterfrom
issue-524

Conversation

@alexander-akait

@alexander-akait alexander-akait commented Jun 6, 2017

Copy link
Copy Markdown
Member

Fixes #524

We can avoid using $schema as we can do in webpack (https://github.com/webpack/webpack/tree/master/schemas)

@codecov

codecov Bot commented Jun 6, 2017

Copy link
Copy Markdown

Codecov Report

Merging #527 into master will decrease coverage by 0.27%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #527      +/-   ##
==========================================
- Coverage   90.38%   90.11%   -0.28%     
==========================================
  Files           6        4       -2     
  Lines         364      354      -10     
  Branches       77       76       -1     
==========================================
- Hits          329      319      -10     
  Misses         35       35
Impacted Files Coverage Δ
index.js 90.95% <100%> (-0.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0e88d0...541ab86. Read the comment docs.

@joshwiens

Copy link
Copy Markdown
Contributor

@bebraw - Wouldn't it just make more sense to migrate this to schema utils and maintain the ajv dependency there?

@bebraw

bebraw commented Jun 6, 2017

Copy link
Copy Markdown
Contributor

@d3viant0ne Agreed.

@evilebottnawi Can you make that change? Better solution over longer term.

@alexander-akait

Copy link
Copy Markdown
Member Author

@bebraw let's do it

@alexander-akait
alexander-akait force-pushed the issue-524 branch 2 times, most recently from 0ec10e6 to 0de03b4 Compare June 6, 2017 20:16
@alexander-akait

Copy link
Copy Markdown
Member Author

/cc @d3viant0ne @bebraw

@michael-ciniawsky michael-ciniawsky changed the title fix: validation schema with latest ajv fix: validation schema with schema-utils Jun 6, 2017
Comment thread index.js Outdated
var loaderUtils = require("loader-utils");
var schemaTester = require('./schema/validator');
var validateOptions = require('schema-utils');
var loaderSchema = require('./schema/loader-schema');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The two schemas can be removed, just pass the path/to/schema.json to validateOptions

Comment thread index.js Outdated
options = { filename: options };
} else {
schemaTester(pluginSchema, options);
validateOptions(pluginSchema, options, ' Extract Text Plugin');

@michael-ciniawsky michael-ciniawsky Jun 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

./schema/plugin.json && Extract Text Plugin => Extract Text Plugin (no leading space)

Please rename plugin-schema.js => plugin.json

Comment thread index.js Outdated
options = { loader: options };
} else {
schemaTester(loaderSchema, options);
validateOptions(loaderSchema, options, 'Extract Text Loader');

@michael-ciniawsky michael-ciniawsky Jun 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

./schema/loader.json && Extract Text Loader => Extract Text Plugin (Loader)

Please rename loader-schema.js => loader.json ⚠️ JS => JSON

Comment thread test/extract.test.js Outdated
},
function(err) {
return err.message === 'data[\'style\'] should NOT have additional properties';
console.log(err.message)

@michael-ciniawsky michael-ciniawsky Jun 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needed for tests or forgotten to remove ? 😛

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@michael-ciniawsky stupid error 😄

@alexander-akait

Copy link
Copy Markdown
Member Author

/cc @michael-ciniawsky

Comment thread index.js Outdated
options = { filename: options };
} else {
schemaTester(pluginSchema, options);
validateOptions('schema/plugin.json', options, 'Extract Text Plugin');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to double check => ./ <= schema... ? (validateOptions use require() 'internally')

Comment thread index.js Outdated
options = { loader: options };
} else {
schemaTester(loaderSchema, options);
validateOptions('schema/loader.json', options, 'Extract Text Plugin (Loader)');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above 😛

@alexander-akait

Copy link
Copy Markdown
Member Author

/cc @michael-ciniawsky done

@michael-ciniawsky
michael-ciniawsky merged commit dfeb347 into master Jun 7, 2017
@michael-ciniawsky
michael-ciniawsky deleted the issue-524 branch June 7, 2017 22:09
@michael-ciniawsky michael-ciniawsky changed the title fix: validation schema with schema-utils fix: validation schema (schema-utils) Jun 7, 2017
@michael-ciniawsky michael-ciniawsky added this to the 2.1.1 milestone Jun 7, 2017
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.

4 participants