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

Update micromatch to the latest version 🚀 #2597

Closed
wants to merge 1 commit into from

Conversation

greenkeeper[bot]
Copy link
Contributor

@greenkeeper greenkeeper bot commented May 29, 2017

Version 3.0.0 of micromatch just got published.

Dependency micromatch
Current Version 2.3.11
Type dependency

The version 3.0.0 is not covered by your current version range.

Without accepting this pull request your project will work just like it did before. There might be a bunch of new features, fixes and perf improvements that the maintainers worked on for you though.

I recommend you look into these changes and try to get onto the latest version of micromatch.
Given that you have a decent test suite, a passing build is a strong indicator that you can take advantage of these changes by merging the proposed change into your project. Otherwise this branch is a great starting point for you to work on the update.


Commits

The new version differs by 75 commits.

There are 75 commits in total.

See the full diff

Not sure how things should work exactly?

There is a collection of frequently asked questions and of course you may always ask my humans.


Your Greenkeeper Bot 🌴

greenkeeper bot added a commit that referenced this pull request May 29, 2017
@greenkeeper
Copy link
Contributor Author

greenkeeper bot commented May 29, 2017

Version 3.0.1 just got published.

Update to this version instead 🚀

Commits

The new version differs by 2 commits.

See the full diff

greenkeeper bot added a commit that referenced this pull request May 31, 2017
@greenkeeper
Copy link
Contributor Author

greenkeeper bot commented May 31, 2017

Version 3.0.2 just got published.

Update to this version instead 🚀

Commits

The new version differs by 5 commits.

See the full diff

@jonschlinkert
Copy link

just checking in to say thanks for using micromatch, and to offer our help if/when you decide to merge this in.

I see that some tests are failing, if micromatch 3.0.0 is indeed the cause, please stop by and let us know on the micromatch issues. thanks!

@hudochenkov
Copy link
Member

@jonschlinkert thank you for offering your help! It will really help to understand what might go wrong if we'll have changelog for a new micromatch version.

@hudochenkov
Copy link
Member

hudochenkov commented May 31, 2017

This test is failing.

And there we're using micromatch.

@ntwb
Copy link
Member

ntwb commented May 31, 2017

The test, pasting here for reference:

it("createLinter().isPathIgnored", () => {
  const config = {
    ignoreFiles: [ "**/*.css", "!**/invalid-hex.css" ],
    rules: { "block-no-empty": true },
  }
  const linter = stylelint.createLinter({ config })

  return Promise.all([ linter.isPathIgnored("a.css"), linter.isPathIgnored("foo/bar/baz.css"), linter.isPathIgnored("foo/bar/baz.scss"), linter.isPathIgnored("foo/invalid-hex.css") ]).then(results => {
    expect(results).toEqual([ true, true, false, false ])
  })
})

The test results:

FAIL  lib/__tests__/createLinter.test.js
  ● createLinter().isPathIgnored
    expect(received).toEqual(expected)
    
    Expected value to equal:
      [true, true, false, false]
    Received:
      [true, true, true, false]
    
    Difference:
    
    - Expected
    + Received
    
     Array [
       true,
       true,
    +  true,
       false,
    -  false,
     ]
      
      at Promise.all.then.results (lib/__tests__/createLinter.test.js:35:21)

I believe our expected test results are accurate i that linter.isPathIgnored("foo/bar/baz.scss") should be false, the path foo/bar/baz.scss isn't included in the ignoreFiles config and as such it shouldn't be ignored and should not return true

@jonschlinkert
Copy link

it will really help to understand what might go wrong if we'll have changelog for a new micromatch version.

There shouldn't be any regressions. If there are it's unexpected.

pasting here for reference:

thanks, that helps!

@jonschlinkert
Copy link

@ntwb or @hudochenkov, if you have a moment could you re-run the tests on appveyor and travis? all tests are passing for me locally, and since npm was having server issues earlier I just want to make sure it's a bug before I dig in. thanks in advance

@ntwb
Copy link
Member

ntwb commented May 31, 2017

@jonschlinkert I've just restarted all the CI jobs

@ntwb
Copy link
Member

ntwb commented May 31, 2017

Looking like 2 out of 3 are passing, both Travis CI jobs are passing but not AppVeyor 🤔

@ntwb
Copy link
Member

ntwb commented May 31, 2017

@jonschlinkert
Copy link

Looks like maybe some incompatibility with globs and Windows?

well, "incompatibility" might be stretching it, there are 36,000 unit tests in micromatch with a great deal of attention focused on Windows paths. my guess is that the same bug is causing all of the failures here. Hopefully, that will be the case. I'll look into it ASAP and push up a fix. hopefully tomorrow


on a side note, I just pulled down stylelint and ran the tests on a Windows 8 machine locally (without upgrading micromatch), and here is what I'm getting:

image

none of these test failures are matching/globbing related. just thought you'd want to know.

@ntwb
Copy link
Member

ntwb commented Jun 2, 2017

"...well, "incompatibility" might be stretching it"

My bad, sorry, not my best choice of words 😄

Thanks for the further info, will also take a closer look myself 👍

@jonschlinkert
Copy link

My bad, sorry, not my best choice of words

lol no worries. Btw I spent some time trying to debug yesterday. Is there a way to run a specific test? It takes 5-6 minutes for all the tests to run, which makes it difficult to re-run tests for just the failing ones. I'm not familiar enough with your test suite, any guidance on that would help. thanks

@hudochenkov
Copy link
Member

Is there a way to run a specific test?

@jonschlinkert yes, run npm run watch, and then press p to filter by a filename regex pattern.

greenkeeper bot added a commit that referenced this pull request Jun 2, 2017
@jonschlinkert
Copy link

thanks. I just noticed that the appveyor tests are against micromatch 3.0.0, can you try to re-run it against the latest micromatch, v3.0.3? I'm not familiar with greenkeeper.

@hudochenkov
Copy link
Member

@jonschlinkert restarted. Still has errors :(

Thank you for working on solution!

@jeddy3
Copy link
Member

jeddy3 commented Jun 29, 2017

Superseded by #2691

@jeddy3 jeddy3 closed this Jun 29, 2017
@jeddy3 jeddy3 deleted the greenkeeper/micromatch-3.0.0 branch June 29, 2017 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants