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

refactor content files import into its own plugin. #30

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tindn
Copy link
Collaborator

@tindn tindn commented Jan 26, 2018

refactor content files import into its own plugin.

update readme for this refactor.

add example config json.

closes #29

update readme for this refactor.

add example config json.
@tindn tindn requested a review from tnguyen14 January 26, 2018 02:13
@coveralls
Copy link

coveralls commented Jan 26, 2018

Coverage Status

Coverage decreased (-16.0%) to 57.143% when pulling 06b4032 on 29-content-parser-plugin into 907e841 on master.

Copy link
Owner

@tnguyen14 tnguyen14 left a comment

Choose a reason for hiding this comment

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

Can we not convert everything from tabs to spaces?

@tindn
Copy link
Collaborator Author

tindn commented Jan 26, 2018

updated the tabs issue.
tests still failing. looking into it.

@@ -1,3 +1,4 @@
node_modules
.DS_Store
.nyc_output
.vscode
Copy link
Owner

Choose a reason for hiding this comment

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

You can add this to your ~/.gitignore so you don't have to add it for every project.

helpersDir: 'templates/helpers'
},
plugins: {
files: {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you discard all the changes in this file except for this part?

@@ -5,14 +5,22 @@ const cpFile = require('cp-file');
function copyImages (opts) {
return new Promise((resolve, reject) => {
// copy images
glob(opts.contentsDir + '/**/*.{jpg,png,svg}', (err, files) => {
glob(opts.files.contentsDir + '/**/*.{jpg,png,svg}', (err, files) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Should this be opts.plugins.files?

Also, it might make sense to have copyImages be part of the files plugin?

}
});
currentDir[file.filename] = file;
return Promise.all(plugins(opts.plugins, {}));
Copy link
Owner

Choose a reason for hiding this comment

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

I think plugins() return a single promise, so no need to do Promise.all here?

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

Successfully merging this pull request may close these issues.

Refactor contents folder parser into a plugin
3 participants