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

Upgrade ignore dependency #112

Closed
wants to merge 1 commit into from
Closed

Conversation

futpib
Copy link
Contributor

@futpib futpib commented Mar 16, 2019

fixes #92

@futpib futpib changed the title Upgrade ignore dependency [WIP] Upgrade ignore dependency Mar 16, 2019
@futpib futpib changed the title [WIP] Upgrade ignore dependency Upgrade ignore dependency Mar 16, 2019
@sindresorhus
Copy link
Owner

Can you describe the changes? Also see #104 for a previous attempt that I don't think is correct.

@futpib
Copy link
Contributor Author

futpib commented Mar 16, 2019

After upgrade you get invalid path errors you showed in #92 (comment). The ignore upgrade guide says they made paths starting with ./ or ../ invalid. I saw that these were introduced by path.relative call in ignores.ignores(slash(path.relative(cwd, p))) and changed it to ignores.ignores(slash(p)). At this point I trusted green tests and submitted the PR.

Now, after a closer look, I see that this almost made sense except for a call like globby.sync('../foo/*', { gitignore: true } and a .gitignore file in cwd, which is not covered by the tests, which would result in ignores call with a path starting with ../.

I might come back to this with a better PR.

@futpib futpib closed this Mar 16, 2019
@fisker fisker mentioned this pull request Feb 26, 2020
4 tasks
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.

Update ignore package to latest version
2 participants