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

Consume most recent task-lib in copyfiles #9390

Closed
wants to merge 8 commits into from

Conversation

damccorm
Copy link

@damccorm damccorm commented Jan 23, 2019

While all other task-lib functions used by DeleteFilesV1 haven't changed significantly, tl.match() has (it used to use semver and now it does its own matching). I think this should resolve #9231 and resolve #9046

EDIT: Also allows broken symlinks, should fix #9768 as well

@damccorm damccorm requested a review from ericsciple January 23, 2019 14:58
@@ -16,7 +16,11 @@ let flattenFolders: boolean = tl.getBoolInput('flattenFolders', false);
// determine the relative path of each found file (substring using sourceFolder.length).
sourceFolder = path.normalize(sourceFolder);

let allPaths: string[] = tl.find(sourceFolder); // default find options (follow sym links)
// Default find options except for allowing broken sym links.
Copy link
Member

Choose a reason for hiding this comment

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

Does this change behavior or keep it the same? Do we have tests to cover this?

Copy link
Author

Choose a reason for hiding this comment

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

The only difference in behavior is that it allows broken symlinks (doesn't throw when it encounters one). We don't have coverage here, we do in the task-lib itself though. IMO it doesn't feel like this task's responsibility to test that specific functionality and testing broken symlinks is kinda gross so I think its fine, thoughts?

@damccorm damccorm requested review from jtpetty and removed request for jtpetty September 19, 2019 14:03
@damccorm
Copy link
Author

Closing this one for a bit since the code has shifted since its been opened, need to readdress

@damccorm damccorm closed this Sep 19, 2019
@damccorm damccorm deleted the users/damccorm/issue-9231 branch January 10, 2020 14:14
@damccorm damccorm restored the users/damccorm/issue-9231 branch January 10, 2020 14:14
@damccorm damccorm deleted the users/damccorm/issue-9231 branch January 10, 2020 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants