Skip to content

Switch to micromatch: fixes yarnpkg/yarn#3336 - #3339

Merged
bestander merged 5 commits into
yarnpkg:masterfrom
jrop:master
May 12, 2017
Merged

Switch to micromatch: fixes yarnpkg/yarn#3336#3339
bestander merged 5 commits into
yarnpkg:masterfrom
jrop:master

Conversation

@jrop

@jrop jrop commented May 6, 2017

Copy link
Copy Markdown
Contributor

Summary

The micromatch library provides an inconsistent implementation of .makeRe(...) with the rest of it's interface. It is known that makeRe does not produce Regular Expressions that can be trusted:

This PR switches from minimatch => micromatch.

Motivation: #3336 😄

Test plan

TODO:

  • Update tests and make them pass (before merge)

Tested so far:

$ alias my_yarn='node /github/yarn/lib/cli/index.js'
$ cd /my/other/project
$ my_yarn pack
$ tar -tf my-package-v1.0.0.tgz
#
# Output is as expected
#

Jonathan Apodaca added 2 commits May 6, 2017 12:44
@jrop

jrop commented May 8, 2017

Copy link
Copy Markdown
Contributor Author

Unpredictable tests on Travis fail with "timeout" errors:

Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
      
      at ontimeout (timers.js:386:14)
      at tryOnTimeout (timers.js:250:5)
      at Timer.listOnTimeout (timers.js:214:5)

This failure does not seem to be related to this PR, am I correct?

Jonathan Apodaca and others added 2 commits May 12, 2017 09:11
@bestander
bestander merged commit d223116 into yarnpkg:master May 12, 2017
@bestander

Copy link
Copy Markdown
Member

Nice, thanks for a thorough explanation, tests and fix

@jrop

jrop commented May 12, 2017

Copy link
Copy Markdown
Contributor Author

@bestander 😰 I just realized that the way I added the extra test causes it to fail on windows in AppVeyor, because I hard coded the path separator lib/a.js for example. On windows the test fails because the path will be lib\a.js. I pushed one more commit that changes the test to use path.join('lib', 'a.js'). Does it need another PR?

@bestander

Copy link
Copy Markdown
Member

Yeah, could you send a new PR?

@bestander

Copy link
Copy Markdown
Member

@jrop when to expect the PR?
I'd like to cut 0.25 release

@jrop

jrop commented May 12, 2017

Copy link
Copy Markdown
Contributor Author

@bestander Just submitted. Sorry about that ☹️

@bestander

Copy link
Copy Markdown
Member

no worries :)

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