-
Notifications
You must be signed in to change notification settings - Fork 83
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
There is no way to include all files in manifest #165
Comments
I agree. I just installed and used this plugin. I thought it is a bug... |
Agreed. I understand adding the option to limit the files in the manifest, but there does need to be a way to disable that filter. I ended up having to get a list of all extensions to pass it in... not a great solution IMHO. var path = require('path');
var gulp = require('gulp');
var revAll = require('gulp-rev-all');
var globExpand = require('glob-expand');
gulp.task('revision', () => {
var src = 'built/**/*.*';
var extensions = globExpand(src).map(path.extName);
return gulp.src(src)
.pipe(revAll({
includeFilesInManifest: extensions
}))
.pipe(gulp.dest('dist'));
}); Wouldn't it be nicer if you could just do this? .pipe(revAll({
includeFilesInManifest: '*' // or null or some other value
})) Are you open to a pull request for this? Looking at the code I think it could be a one-liner. |
One could patch this as follows: Modify Lines 404 to 407 in 7fc6134
// Add only specific file types to the manifest file
if (Array.isArray(this.options.includeFilesInManifest) && this.options.includeFilesInManifest.indexOf(ext) !== -1) {
this.manifest[pathOriginal] = pathRevisioned;
} And then just set |
After #151 landed, file extensions are strictly opt-in to include them in the manifest. As some of the discussion in the PR mentioned, this is unintuitive as files are renamed but not included in the manifest without any explanation.
The default behaviour was to include all file types in the manifest, and people were happy with that. I'm proposing we bring that default behaviour back with some way of opting in to the behaviour that #151 introduced.
The text was updated successfully, but these errors were encountered: