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 for working in subdir #78

Merged
merged 2 commits into from
Oct 17, 2016
Merged

Fix for working in subdir #78

merged 2 commits into from
Oct 17, 2016

Conversation

TheWolfNL
Copy link
Contributor

Fix so the paths will be properly resolved and working when the git directory differs from the current working directory

…irectory differs from the current working directory
@@ -5,6 +5,6 @@ const path = require('path')
module.exports = function resolvePaths(filePaths, relativeTo) {
return filePaths.map((file) => {
if (!relativeTo) relativeTo = process.cwd() // eslint-disable-line
return path.resolve(process.cwd(), relativeTo, file.filename)
return path.resolve(process.cwd(), path.relative(process.cwd(), relativeTo), file.filename)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need the first process.cwd() here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you also add a test and update the README for that?

@@ -35,7 +35,7 @@ cosmiconfig('lint-staged', {
console.error(err)
}

const tasks = generateTasks(config, resolvePaths(files))
const tasks = generateTasks(config, resolvePaths(files, sgf.cwd))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we set the cwd as a constant in the beginning and reuse? Don't like the coupling with another lib here.

cleanup path resolving

Todo: create test and run tests
Copy link
Collaborator

@okonet okonet left a comment

Choose a reason for hiding this comment

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

Awesome!

@okonet
Copy link
Collaborator

okonet commented Oct 14, 2016

@TheWolfNL It says "WIP" — are you still doing some more work on this?

@TheWolfNL
Copy link
Contributor Author

i'm planning on creating a test this weekend, didn't have time to test this yet.

@TheWolfNL
Copy link
Contributor Author

@okonet I thought there were no tests written for the nested config yet, but apparently you've already wrote those 👍
I've tested these changes and they're ready to be merged 😄

@okonet okonet merged commit c095272 into lint-staged:master Oct 17, 2016
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.

2 participants