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: make sure deleted files aren't restored due to git bugs #778

Merged
merged 7 commits into from
Jan 30, 2020

Conversation

iiroj
Copy link
Member

@iiroj iiroj commented Jan 25, 2020

Fixes #561

Currently, version 10 only fixes some of the issues with deleted files getting restored when running lint-staged. This PR adds yet more checks to fix the remaining issues. The issue itself seems to be with the way git tracks files, and you can read more here in Stack Overflow.

I also updated some eslint rules to take into account the Node.js version bump, and after that refactored the file api to use fs.promises to simplify the code.

@codecov
Copy link

codecov bot commented Jan 25, 2020

Codecov Report

Merging #778 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #778   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     13           
  Lines         485    486    +1     
  Branches      106    107    +1     
=====================================
+ Hits          485    486    +1
Impacted Files Coverage Δ
lib/index.js 100% <ø> (ø) ⬆️
lib/getStagedFiles.js 100% <ø> (ø) ⬆️
lib/resolveGitRepo.js 100% <100%> (ø) ⬆️
lib/gitWorkflow.js 100% <100%> (ø) ⬆️
lib/file.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b98a5ed...0121e8b. Read the comment docs.

Copy link

@tremby tremby left a comment

Choose a reason for hiding this comment

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

I've given this a go, and it's working for all the tests I ran.

lib/gitWorkflow.js Outdated Show resolved Hide resolved
lib/gitWorkflow.js Outdated Show resolved Hide resolved
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.

Some minor changes and it’s good to go.

lib/file.js Outdated Show resolved Hide resolved
lib/file.js Outdated Show resolved Hide resolved
lib/file.js Outdated Show resolved Hide resolved
lib/file.js Outdated Show resolved Hide resolved
lib/gitWorkflow.js Show resolved Hide resolved
lib/gitWorkflow.js Outdated Show resolved Hide resolved
lib/gitWorkflow.js Outdated Show resolved Hide resolved
test/runAll.unmocked.spec.js Outdated Show resolved Hide resolved
@iiroj iiroj force-pushed the fix-deleted branch 2 times, most recently from a6ece6f to 892d706 Compare January 28, 2020 15:29
@iiroj
Copy link
Member Author

iiroj commented Jan 28, 2020

The test coverage decrease is due to current git versions not hitting the special case. I'll have to extract it back into a separate function so that it can be tested via a mock.

EDIT: Handled via a synthetic test to see that the method deletes untracked files when run.

@iiroj iiroj requested a review from okonet January 28, 2020 15:53
okonet
okonet previously approved these changes Jan 28, 2020
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.

I still think await would be more appropriate but I leave to you.

@iiroj
Copy link
Member Author

iiroj commented Jan 29, 2020

@tremby If you don't mind, could you test this again after the rebase? There were some other related changes put into master.

@iiroj
Copy link
Member Author

iiroj commented Jan 30, 2020

What about this, @okonet?

@iiroj iiroj merged commit 6bfbe6c into master Jan 30, 2020
@iiroj iiroj deleted the fix-deleted branch January 30, 2020 13:30
@okonet
Copy link
Collaborator

okonet commented Jan 30, 2020

🎉 This PR is included in version 10.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tremby
Copy link

tremby commented Feb 3, 2020

@tremby If you don't mind, could you test this again after the rebase? There were some other related changes put into master.

Late response but I've tested again with the latest release. Looks good. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Deleted files restored after committing with no files matching lint-staged config
3 participants