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.

Sign up for GitHub

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

Jump to bottom

fix: copy ignored tracked files #1074

Merged
merged 11 commits into from
Apr 4, 2022
Merged

fix: copy ignored tracked files #1074

merged 11 commits into from
Apr 4, 2022

Conversation

ChristopherHX
Copy link
Contributor

@ChristopherHX ChristopherHX commented Mar 24, 2022 •
edited

Resolves #912
Resolves #1002
Closes #949

What I have tested:

  • copy without git directory in current folder => old behavior
  • copy with git directory, now includes all tracked files even those which are ignored
  • first level of submodules
  • you can now cancel the copydir by pressing ctrl+c

I'm not shure how to write tests for this change.

ChristopherHX
ChristopherHX commented Mar 24, 2022
View reviewed changes
pkg/container/docker_run.go
Comment on lines -659 to -667
// manually close here after each file operation; deferring would cause each file close
// to wait until all operations have completed.
f.Close()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment is no longer valid, because the code block is wrapped by a function. Defer is executed directly after it returns to filepath.Walk and before it goes on with the next entry.

@ChristopherHX
Copy link
Contributor Author

ChristopherHX commented Mar 24, 2022 •
edited

@jsoref You might want to check if this fits your usecase.

act -w should be changed in a later PR.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 24, 2022 •
edited

MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ CREDENTIALS secretlint yes no 0.98s
✅ EDITORCONFIG editorconfig-checker 3 0 0.59s
✅ GIT git_diff yes no 0.01s

See errors details in artifact MegaLinter reports on CI Job page
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

@codecov
Copy link

codecov bot commented Mar 24, 2022 •
edited

Codecov Report

Merging #1074 (a01d2cd) into master (4f8da0a) will increase coverage by 2.50%.
The diff coverage is 79.22%.

@@            Coverage Diff             @@
##           master    #1074      +/-   ##
==========================================
+ Coverage   57.50%   60.01%   +2.50%     
==========================================
  Files          32       39       +7     
  Lines        4594     4924     +330     
==========================================
+ Hits         2642     2955     +313     
+ Misses       1729     1727       -2     
- Partials      223      242      +19
Impacted Files Coverage Δ
pkg/model/action.go 0.00% <ø> (ø)
pkg/model/planner.go 50.73% <ø> (+0.32%) ⬆️
pkg/model/workflow.go 50.91% <ø> (ø)
pkg/container/docker_run.go 5.11% <2.50%> (-0.43%) ⬇️
pkg/container/file_collector.go 44.85% <44.85%> (ø)
pkg/runner/logger.go 66.00% <67.56%> (+0.56%) ⬆️
pkg/runner/runner.go 73.72% <73.68%> (-2.75%) ⬇️
pkg/exprparser/interpreter.go 74.57% <75.00%> (+1.17%) ⬆️
pkg/runner/expression.go 91.02% <78.26%> (+0.20%) ⬆️
pkg/runner/step.go 80.76% <80.76%> (ø)
... and 18 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@ChristopherHX ChristopherHX marked this pull request as ready for review March 25, 2022 15:48
@ChristopherHX ChristopherHX requested a review from a team as a code owner March 25, 2022 15:48
@mergify mergify bot requested a review from a team March 29, 2022 15:19
@ChristopherHX
refactor: Extract callback to fileCollector
6241229 
fix: temporary tar archive not deletable on windows
fix: `.*` in gitignore ignores all files
@mergify

This comment was marked as resolved.

Sign in to view
@mergify mergify bot added the needs-work Extra attention is needed label Mar 29, 2022
@mergify

This comment was marked as duplicate.

Sign in to view
@mergify mergify bot removed the needs-work Extra attention is needed label Mar 29, 2022
@mergify
Copy link
Contributor

mergify bot commented Mar 30, 2022

@ChristopherHX this pull request has failed checks 🛠

@mergify mergify bot added the needs-work Extra attention is needed label Mar 30, 2022
@mergify mergify bot removed the needs-work Extra attention is needed label Mar 30, 2022
ZauberNerd
ZauberNerd reviewed Mar 31, 2022
View reviewed changes
Copy link
Contributor

@ZauberNerd ZauberNerd left a comment

Choose a reason for hiding this comment

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

Overall pretty good. I'll approve once the requested changes by @KnisterPeter / @catthehacker are committed.

pkg/container/file_collector_test.go Show resolved Hide resolved
@mergify mergify bot requested a review from a team March 31, 2022 15:49
@mergify
Copy link
Contributor

mergify bot commented Apr 4, 2022

@ChristopherHX this pull request has failed checks 🛠

@mergify mergify bot added needs-work Extra attention is needed and removed needs-work Extra attention is needed labels Apr 4, 2022
@mergify mergify bot merged commit c27e079 into master Apr 4, 2022
@mergify mergify bot deleted the fix/copyignoredtrackedfiles branch April 4, 2022 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZauberNerd ZauberNerd ZauberNerd approved these changes

@KnisterPeter KnisterPeter KnisterPeter approved these changes

@catthehacker catthehacker catthehacker approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
4 participants
@ChristopherHX @ZauberNerd @KnisterPeter @catthehacker