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

fix(pattern-validation-error): Throw error when a file is not matching the pattern #108

Closed
wants to merge 3 commits into from

Conversation

yannvr
Copy link

@yannvr yannvr commented Jan 19, 2017

It's a trivial logging addition to disclose when a migration task file pattern is not matching the default one. It seems to be a common gotcha that would save a lot of time to Umzug adopters because invalid file patterns are silently ignored.

jukkah added a commit that referenced this pull request Apr 22, 2017
@yannvr pointed out in #108 that if there is some extra files in migrations
directory, it might be a sign of invalid pattern. In the most common use case,
there is only migrations in migrations directory. => If there is other files,
the pattern is probably filtering out some migrations.
@@ -416,6 +416,9 @@ var Umzug = module.exports = redefine.Class(/** @lends Umzug.prototype */ {
.promisify(fs.readdir)(this.options.migrations.path)
.bind(this)
.filter(function (file) {
if(!this.options.migrations.pattern.test(file)) {
throw new Error('File: ' + file + ' does not match pattern: ' + this.options.migrations.pattern);
Copy link
Contributor

Choose a reason for hiding this comment

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

Throw doesn't work as it breaks this down if there is at least one file that doesn't match to the pattern. It should be some kind of console.warn() instead.

@jukkah
Copy link
Contributor

jukkah commented Apr 22, 2017

I've made much refactoring in the code base or at least git thinks so. To help you, I rewrite this in 890e5e1 so that you don't need to update your code.

@jukkah jukkah closed this Apr 22, 2017
jukkah added a commit to jukkah/umzug that referenced this pull request Apr 24, 2017
jukkah added a commit that referenced this pull request Apr 24, 2017
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.

2 participants