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

fixed possible replace-bug, formatting, PSv2 compat #12

Merged
merged 3 commits into from
Oct 25, 2018
Merged

fixed possible replace-bug, formatting, PSv2 compat #12

merged 3 commits into from
Oct 25, 2018

Conversation

jantari
Copy link
Contributor

@jantari jantari commented Oct 25, 2018

Proposed changes:

Proposed changes:

- Replaced backtick line continuations with the pipe which naturally supports multi-line (https://get-powershellblog.blogspot.com/2017/07/bye-bye-backtick-natural-line.html#pipelineop)
- Replaced aliases with Full cmdlet names
- Replaced the `Where-Object` comparison statement with a scriptblock as comparison statements are new in PowerShell v3 and thus will not work on Windwos 7/Server 2008 out of the box (https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/where-object?view=powershell-6)
- Added "OneDrive" to the excluded folders list
- Removed `Foreach-Object` (`%`) loop as `Copy-Item` natively accepts a collection of files as pipeline input
- Switched `.Replace($item.BaseName,"")` which was dangerous because if the basename occurs multiple times in a path such as `"C:\Users\Henry\Pictures\Henry.jpg"` all occurrences would have been removed with safer `$item.Parent.FullName`
- Introduced constant array for holding folder names to be excluded
Copy link
Owner

@guitarrapc guitarrapc left a comment

Choose a reason for hiding this comment

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

Can you check suggesting fix? Others seems very well.

@guitarrapc
Copy link
Owner

Thank you for the PR. I appreciate a lot about considering much nice way:)
Can you review my comment?

guitarrapc and others added 2 commits October 25, 2018 17:43
Co-Authored-By: jantari <jantari@live.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-Authored-By: jantari <jantari@live.com>
@guitarrapc guitarrapc merged commit 8c72949 into guitarrapc:master Oct 25, 2018
@guitarrapc
Copy link
Owner

thank you for your check. Merge it:)

@guitarrapc guitarrapc self-assigned this Oct 25, 2018
@jantari
Copy link
Contributor Author

jantari commented Oct 25, 2018

Thanks for the quick responses and merge :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants